On 20 October 2015 at 12:08, Savolainen, Petri (Nokia - FI/Espoo) <
[email protected]> wrote:
> // Check that everything is supported
>
> // 1: all is supported, 0: some stats missing
>
> int odp_pktio_stats_capability_all(odp_pktio_stats_capability_t
> *capability);
>
> And what is the actual use case of this function?
>
>
>
> If everything is supported (the common case), user does not have to check
> any further. It’s also forward compatible way to check that everything is
> supported. Without this helper call, user needs to check every bit (or
> #define) individually and maintain the check code over API versions.
>
> Now you are creating multiple control paths in the application. Someone
> could be using this function and only support the case that all stats
> counters are supported. Then running on some other platform, all counters
> will not be supported and another path through the code (that checks
> whether the individual counters used are supported) must be taken. This
> path might not exist (because it was not needed) or might not work
> correctly (because never tested).
>
>
>
> Just provide one way of doing this. Better the return the supported
> counters as a bitmask (uint32_t). The caller can check (e.g. using
> bit-wise and operation) whether (all of) the counters of interest are
> supported.
>
>
>
> all == 0 works up to 32/64 counters (after that it would be all[0] == 0 &&
> all[1] == 0), which maybe a problem in the future. There are e.g. counters
> defined for different rx and tx packet sizes, etc. These counter may be
> many. When capability is per “get stats call”, in principle user needs to
> check every time if e.g. all 40 counters are still there, before reading
> those counters.
>
>
>
> You can have e.g. two functions (code paths) one for “all supported”
> another for “some missing”. The first does not have any if’s in fast path,
> where as the second is full of those
>
Is this an actual use case you need to optimise?
If it is only hypothetical, I think this optimisation is premature.
>
>
> func_all_supported()
>
> {
>
> use counter.x;
>
> use counter.y;
>
> }
>
>
>
> func_some_missing()
>
> {
>
> if (capability.x)
>
> use counter.x;
>
> if (capability.y)
>
> use counter.y;
>
> }
>
>
>
>
>
> -Petri
>
>
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
