Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
On Thu, Mar 08, 2018 at 22:07:35 +0300, Vladimir Sementsov-Ogievskiy wrote: > 08.03.2018 21:56, Emilio G. Cota wrote: > > * Binning happens only at print time, so that we retain the flexibility to > > * choose the binning. This might not be ideal for workloads that do not > > care > > * much about precision and insert many samples all with different x values; > > * in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1) > > * should be considered. (snip) > In this case, I'll have to do same bin search (and store same interval > settings) as I already do, on my part, to calculate a parameter for qdist > interface. And I'll have store almost all same data on my part. So, it > doesn't really help. And I need nothing of qdist benefits: I don't need (and > don't want) dynamic allocation of bins on adding an element or any type of > visualization. I see. You require a couple of features that qdist doesn't yet support: - Arbitrarily-sized, pre-defined bins. - Support for querying the data programmatically instead of just printing it out. We could circumvent the first missing feature with pre-binning, but in that case we'd do a bsearch twice as you point out (BTW your concern about memory allocation wouldn't apply though). The second missing feature should be easy to add to qdist. That said, given that you want this in for 2.12, I'd go with your approach for now. In the future we should look into supporting your use case in qdist, since it is likely that there will be more users with a similar need. Thanks, Emilio
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
08.03.2018 21:56, Emilio G. Cota wrote:
On Thu, Mar 08, 2018 at 14:42:29 +0300, Vladimir Sementsov-Ogievskiy wrote:
Hi Emilio!
Looked through qdist, if I understand correctly, it saves each added (with
different value) element. It is not effective for disk io timing - we'll
have too much elements. In my approach, histogram don't grow, it initially
have several ranges and counts hits to each range.
I thought about this use case, i.e. having a gazillion elements.
You should just do some pre-binning before inserting samples
into qdist, as pointed out in this comment in qdist.h:
/*
* Samples with the same 'x value' end up in the same qdist_entry,
* e.g. inc(0.1) and inc(0.1) end up as {x=0.1, count=2}.
*
* Binning happens only at print time, so that we retain the flexibility to
* choose the binning. This might not be ideal for workloads that do not care
* much about precision and insert many samples all with different x values;
* in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1)
* should be considered.
*/
struct qdist_entry {
double x;
unsigned long count;
};
Let me know if you need help with that.
Thanks,
Emilio
In this case, I'll have to do same bin search (and store same interval
settings) as I already do, on my part, to calculate a parameter for
qdist interface. And I'll have store almost all same data on my part.
So, it doesn't really help. And I need nothing of qdist benefits: I
don't need (and don't want) dynamic allocation of bins on adding an
element or any type of visualization.
--
Best regards,
Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
On Thu, Mar 08, 2018 at 14:42:29 +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi Emilio!
>
> Looked through qdist, if I understand correctly, it saves each added (with
> different value) element. It is not effective for disk io timing - we'll
> have too much elements. In my approach, histogram don't grow, it initially
> have several ranges and counts hits to each range.
I thought about this use case, i.e. having a gazillion elements.
You should just do some pre-binning before inserting samples
into qdist, as pointed out in this comment in qdist.h:
/*
* Samples with the same 'x value' end up in the same qdist_entry,
* e.g. inc(0.1) and inc(0.1) end up as {x=0.1, count=2}.
*
* Binning happens only at print time, so that we retain the flexibility to
* choose the binning. This might not be ideal for workloads that do not care
* much about precision and insert many samples all with different x values;
* in that case, pre-binning (e.g. entering both 0.115 and 0.097 as 0.1)
* should be considered.
*/
struct qdist_entry {
double x;
unsigned long count;
};
Let me know if you need help with that.
Thanks,
Emilio
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
On 03/06/2018 10:00 AM, Stefan Hajnoczi wrote: On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: v2: 01: add block_latency_histogram_clear() 02: fix spelling (sorry =() some rewordings remove histogram if latency parameter unspecified Vladimir Sementsov-Ogievskiy (2): block/accounting: introduce latency histogram qapi: add block latency histogram interface qapi/block-core.json | 73 +- include/block/accounting.h | 9 + block/accounting.c | 97 ++ block/qapi.c | 31 +++ blockdev.c | 19 + 5 files changed, 228 insertions(+), 1 deletion(-) The feature looks good. I posted documentation and code readability suggestions. I also think it makes sense; if a v3 hits the list in time, I will probably include it in my last QAPI pull before softfreeze. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
Hi Emilio! Looked through qdist, if I understand correctly, it saves each added (with different value) element. It is not effective for disk io timing - we'll have too much elements. In my approach, histogram don't grow, it initially have several ranges and counts hits to each range. 06.03.2018 20:49, Emilio G. Cota wrote: On Tue, Mar 06, 2018 at 16:00:17 +, Stefan Hajnoczi wrote: On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: Vladimir Sementsov-Ogievskiy (2): block/accounting: introduce latency histogram qapi: add block latency histogram interface (snip) 5 files changed, 228 insertions(+), 1 deletion(-) The feature looks good. I posted documentation and code readability suggestions. Hi Vladimir, Did you consider using qdist? For reference, see commit bf3afd5f419 and/or grep 'qdist'. Thanks, Emilio -- Best regards, Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
On Tue, Mar 06, 2018 at 16:00:17 +, Stefan Hajnoczi wrote: > On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > Vladimir Sementsov-Ogievskiy (2): > > block/accounting: introduce latency histogram > > qapi: add block latency histogram interface (snip) > > 5 files changed, 228 insertions(+), 1 deletion(-) > > The feature looks good. I posted documentation and code readability > suggestions. Hi Vladimir, Did you consider using qdist? For reference, see commit bf3afd5f419 and/or grep 'qdist'. Thanks, Emilio
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
On Wed, Feb 07, 2018 at 03:50:35PM +0300, Vladimir Sementsov-Ogievskiy wrote: > v2: > > 01: add block_latency_histogram_clear() > 02: fix spelling (sorry =() > some rewordings > remove histogram if latency parameter unspecified > > Vladimir Sementsov-Ogievskiy (2): > block/accounting: introduce latency histogram > qapi: add block latency histogram interface > > qapi/block-core.json | 73 +- > include/block/accounting.h | 9 + > block/accounting.c | 97 > ++ > block/qapi.c | 31 +++ > blockdev.c | 19 + > 5 files changed, 228 insertions(+), 1 deletion(-) The feature looks good. I posted documentation and code readability suggestions. Stefan signature.asc Description: PGP signature
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
looks strange and unrelated. 07.02.2018 16:20, no-re...@patchew.org wrote: Hi, This series failed build test on ppc host. Please find the details below. Type: series Subject: [Qemu-devel] [PATCH v2 0/2] block latency histogram Message-id: 20180207125037.13510-1-vsement...@virtuozzo.com === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo "=== ENV ===" env echo "=== PACKAGES ===" rpm -qa echo "=== TEST BEGIN ===" INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --prefix=$INSTALL make -j100 # XXX: we need reliable clean up # make check -j100 V=1 make install === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 error: RPC failed; result=18, HTTP code = 200 fatal: The remote end hung up unexpectedly error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 Traceback (most recent call last): File "/home/patchew/patchew/patchew-cli", line 504, in test_one git_clone_repo(clone, r["repo"], r["head"], logf) File "/home/patchew/patchew/patchew-cli", line 48, in git_clone_repo stdout=logf, stderr=logf) File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org -- Best regards, Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/2] block latency histogram
Hi, This series failed build test on ppc host. Please find the details below. Type: series Subject: [Qemu-devel] [PATCH v2 0/2] block latency histogram Message-id: 20180207125037.13510-1-vsement...@virtuozzo.com === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo "=== ENV ===" env echo "=== PACKAGES ===" rpm -qa echo "=== TEST BEGIN ===" INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --prefix=$INSTALL make -j100 # XXX: we need reliable clean up # make check -j100 V=1 make install === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 error: RPC failed; result=18, HTTP code = 200 fatal: The remote end hung up unexpectedly error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 Traceback (most recent call last): File "/home/patchew/patchew/patchew-cli", line 504, in test_one git_clone_repo(clone, r["repo"], r["head"], logf) File "/home/patchew/patchew/patchew-cli", line 48, in git_clone_repo stdout=logf, stderr=logf) File "/usr/lib64/python3.4/subprocess.py", line 558, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
