Re: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement

["Trahe, Fiona" <fiona.trahe@intel.com>]

Hi Shally, Tomasz,

> > >> >> >> >+       /* Window size */
> > >> >> >> >+       if (test_data->window_sz != -1) {
> > >> >> >> >+               if (param_range_check(test_data->window_sz,
> > >> >> >> >+ &cap->window_size)
> > >> >> >> What if cap->window_size is 0 i.e. implementation default?
> > >> >> >
> > >> >> >TJ: You probably mean cap->window_size.increment = 0 (because
> > >> >> >cap->window_size is a structure). In that case we check if
> > >> >> >test_data->window_sz >=min and test_data->window_sz <= max
> > only,
> > >> >> because increment = 0 means (base on compression API) we have only
> > >> >> one value of windows_size (no range is supported).
> > >> >> But PMD can set min and max too 0 for such case.
> > >> >
> > >> >TJ: I can't see any issue in that case too. Maybe I don't understand
> > >> >what you
> > >> mean but the logic is as follow:
> > >> >1)  if you pass '--window-sz  ...' param. into command line your
> > >> >intention is to force that value of window size during test. We
> > >> >check is this
> > >> value is allow (by param_range_check() function).
> > >> >2) if you plan to use default value - just don't pass '--window-sz'
> > >> >param. in command line at all. In that case we get windows size from
> > >> >window_size.max field, so if window_size.min= window_size.max=0
> > >> test_data->window_sz will be zero, as well.
> > >> >If you mean that behavior is not good - I will be grateful for other
> > >> suggestions.
> > >>
> > >> This is fine. but I am thinking of 3rd case here:
> > >> c) user pass window sz but PMD window_sz.min = max = 0, then user
> > >> requested windowsz is not applicable right?!
> > >
> > >In that case - true. There'll be fail :
> > >"Compress device does not support this window size\n"); So what is your
> > >proposal for  that case?
> > >
> > We can set to window size to implementation default and add in diagnostic
> > of used window sz for test run.
> > No need to fail here I believe.

[Fiona] For Window size capability reported by the PMD in the info struct 
it is not valid to report min=0, max=0. The PMD must report the range it can
handle - the API doesn't suggest otherwise. 
On the xform a specific window size is requested of the PMD, if it doesn't support
this it's allowed to fall back to a lower size according to the API.
However that doesn't mean the PMD can pick any size if it doesn't support the
requested size, i.e. it can't pick a bigger size, just a smaller one. 
If an application requests a smaller window size
than a PMD supports, it can be that the decompression engine
will be unable to decompress if a larger window is used, so the PMD 
should only fall back to a smaller size.
Based on above, I think the perf tool behaviour is ok. 
It should pass the user requested value to the PMD if the PMD capabilities support it.
If not it should fail. If the user wants to measure with a different window size they can
pass in that parameter.
The functional test suite can be used to validate the case where the PMD
falls back - this is not what the perf tool is for.
Does this make sense?

@Shally, do you think we need an API change to support an unlimited set of window sizes?
If so can you explain why?