DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Verma, Shally" <Shally.Verma@cavium.com>
To: "Trahe, Fiona" <fiona.trahe@intel.com>,
	"Jozwiak, TomaszX" <tomaszx.jozwiak@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"akhil.goyal@nxp.com" <akhil.goyal@nxp.com>
Subject: Re: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance	measurement
Date: Mon, 12 Nov 2018 04:45:26 +0000	[thread overview]
Message-ID: <SN6PR07MB5152CA2B1A96C90075404A06F0C10@SN6PR07MB5152.namprd07.prod.outlook.com> (raw)
In-Reply-To: <348A99DA5F5B7549AA880327E580B435896741A1@IRSMSX101.ger.corp.intel.com>



>-----Original Message-----
>From: Trahe, Fiona <fiona.trahe@intel.com>
>Sent: 10 November 2018 06:24
>To: Jozwiak, TomaszX <tomaszx.jozwiak@intel.com>; Verma, Shally <Shally.Verma@cavium.com>; dev@dpdk.org;
>akhil.goyal@nxp.com
>Cc: Trahe, Fiona <fiona.trahe@intel.com>
>Subject: RE: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement
>
>External Email
>
>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.
Agree to this. However my point is what if PMD just leave these window sz values as 0, meaning implementation default i.e.
internally used fixed value used by PMD to lookup for both compression/decompression. But if we are not supporting  window sz = 0 on an API then its fine , no need to handle this special case. However given that, we need to add comment in capability field, PMD must set it to some non-zero value and 0 is not valid case to handle.

>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?
No.I don't intend to add support something like unlimited window sz as that isn't a known use-case. Also, I didn't mean window sz = 0  to be interpreted as unlimited window sz. I just meant 0 = implementation default window sz , if that's supported on compression spec. 

Thanks
Shally
>

  reply	other threads:[~2018-11-12  4:45 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-01 13:27 [dpdk-dev] [PATCH 0/3] app: add initial version of compress-perf Tomasz Jozwiak
2018-10-01 13:27 ` [dpdk-dev] [PATCH 1/3] app/compress-perf: add parser Tomasz Jozwiak
2018-10-01 13:27 ` [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-10-12 10:15   ` Verma, Shally
2018-10-15 15:10     ` Daly, Lee
2018-10-16  5:18       ` Verma, Shally
2018-10-17 14:33       ` Trahe, Fiona
2018-10-17 15:42         ` Verma, Shally
2018-10-17 16:45           ` Trahe, Fiona
2018-10-17 16:47             ` Verma, Shally
2018-11-30 14:43               ` Jozwiak, TomaszX
2018-12-02  6:39                 ` Verma, Shally
2018-12-05  8:51                   ` Jozwiak, TomaszX
2018-11-02  9:59     ` Jozwiak, TomaszX
2018-11-05  8:34       ` Verma, Shally
2018-11-06  8:04         ` Jozwiak, TomaszX
2018-11-06  8:15           ` Verma, Shally
2018-11-06  9:05             ` Jozwiak, TomaszX
2018-11-06 15:39               ` Verma, Shally
2018-11-07 10:18                 ` Jozwiak, TomaszX
2018-11-10  0:54                   ` Trahe, Fiona
2018-11-12  4:45                     ` Verma, Shally [this message]
2018-10-01 13:27 ` [dpdk-dev] [PATCH 3/3] doc/guides/tools: add doc file Tomasz Jozwiak
2018-10-16  9:26   ` Kovacevic, Marko
2018-11-02  9:43 ` [dpdk-dev] [PATCH v2 0/3] add initial version of compress-perf Tomasz Jozwiak
2018-11-02  9:44   ` [dpdk-dev] [PATCH v2 1/3] app/compress-perf: add parser Tomasz Jozwiak
2018-11-05  8:40     ` Verma, Shally
2018-11-06  8:30       ` Jozwiak, TomaszX
2018-11-06  8:32         ` Verma, Shally
2018-11-02  9:44   ` [dpdk-dev] [PATCH v2 2/3] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-11-05  8:56     ` Verma, Shally
2018-11-06  8:49       ` Jozwiak, TomaszX
2018-11-06 15:37         ` Verma, Shally
2018-11-07 10:14           ` Jozwiak, TomaszX
2018-11-02  9:44   ` [dpdk-dev] [PATCH v2 3/3] doc/guides/tools: add doc files Tomasz Jozwiak
2018-11-05  8:57     ` Verma, Shally
2018-11-06  8:51       ` Jozwiak, TomaszX
2018-11-02 11:04   ` [dpdk-dev] [PATCH v2 0/3] add initial version of compress-perf Bruce Richardson
2018-11-02 11:12     ` Jozwiak, TomaszX
2018-11-23 13:06   ` [dpdk-dev] [PATCH v3 0/5] " Tomasz Jozwiak
2018-11-23 13:06     ` [dpdk-dev] [PATCH v3 1/5] app/compress-perf: add parser Tomasz Jozwiak
2018-11-23 13:06     ` [dpdk-dev] [PATCH v3 2/5] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-11-23 13:06     ` [dpdk-dev] [PATCH v3 3/5] doc/guides/tools: add doc files Tomasz Jozwiak
2018-11-23 14:52       ` Varghese, Vipin
2018-11-23 14:59         ` Jozwiak, TomaszX
2018-11-23 13:06     ` [dpdk-dev] [PATCH v3 4/5] app/compress-perf: add dynamic compression test Tomasz Jozwiak
2018-11-23 13:06     ` [dpdk-dev] [PATCH v3 5/5] app/compress-perf: code refactoring Tomasz Jozwiak
2018-11-23 14:27     ` [dpdk-dev] [PATCH v4 0/5] add initial version of compress-perf Tomasz Jozwiak
2018-11-23 14:27       ` [dpdk-dev] [PATCH v4 1/5] app/compress-perf: add parser Tomasz Jozwiak
2018-11-23 15:10         ` Varghese, Vipin
2018-11-23 15:24           ` Bruce Richardson
2018-11-23 15:42             ` Jozwiak, TomaszX
2018-11-23 14:27       ` [dpdk-dev] [PATCH v4 2/5] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-11-23 14:27       ` [dpdk-dev] [PATCH v4 3/5] doc/guides/tools: add doc files Tomasz Jozwiak
2018-11-23 15:00         ` Varghese, Vipin
2018-11-23 15:12           ` Jozwiak, TomaszX
2018-11-23 15:26             ` Varghese, Vipin
2018-11-23 14:27       ` [dpdk-dev] [PATCH v4 4/5] app/compress-perf: add dynamic compression test Tomasz Jozwiak
2018-11-23 14:27       ` [dpdk-dev] [PATCH v4 5/5] app/compress-perf: code refactoring Tomasz Jozwiak
2018-12-05  8:47       ` [dpdk-dev] [PATCH v5 0/5] add initial version of compress-perf Tomasz Jozwiak
2018-12-05  8:47         ` [dpdk-dev] [PATCH v5 1/5] app/compress-perf: add parser Tomasz Jozwiak
2018-12-05  8:47         ` [dpdk-dev] [PATCH v5 2/5] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-12-05  8:47         ` [dpdk-dev] [PATCH v5 3/5] doc/guides/tools: add doc files Tomasz Jozwiak
2018-12-05  8:47         ` [dpdk-dev] [PATCH v5 4/5] app/compress-perf: add dynamic compression test Tomasz Jozwiak
2018-12-05  8:47         ` [dpdk-dev] [PATCH v5 5/5] app/compress-perf: code refactoring Tomasz Jozwiak
2018-12-05 15:37         ` [dpdk-dev] [PATCH v5 0/5] add initial version of compress-perf Daly, Lee
2018-12-12 11:54         ` [dpdk-dev] [PATCH v6 " Tomasz Jozwiak
2018-12-17 11:11           ` Verma, Shally
2018-12-12 12:08         ` Tomasz Jozwiak
2018-12-12 12:08           ` [dpdk-dev] [PATCH v6 1/5] app/compress-perf: add parser Tomasz Jozwiak
2018-12-12 12:08           ` [dpdk-dev] [PATCH v6 2/5] app/compress-perf: add performance measurement Tomasz Jozwiak
2018-12-12 12:08           ` [dpdk-dev] [PATCH v6 3/5] doc/guides/tools: add doc files Tomasz Jozwiak
2018-12-12 12:08           ` [dpdk-dev] [PATCH v6 4/5] app/compress-perf: add dynamic compression test Tomasz Jozwiak
2018-12-12 12:08           ` [dpdk-dev] [PATCH v6 5/5] app/compress-perf: code refactoring Tomasz Jozwiak
2018-12-14 19:24           ` [dpdk-dev] [PATCH v6 0/5] add initial version of compress-perf Trahe, Fiona
2018-12-18 10:28           ` Akhil Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN6PR07MB5152CA2B1A96C90075404A06F0C10@SN6PR07MB5152.namprd07.prod.outlook.com \
    --to=shally.verma@cavium.com \
    --cc=akhil.goyal@nxp.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=tomaszx.jozwiak@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).