| Message ID | 20210304101738.20248-9-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | simplebench improvements | expand |
On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote: > It probably may improve reliability of results when testing in cached > mode. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > scripts/simplebench/bench_block_job.py | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py > index 4f03c12169..fa45ad2655 100755 > --- a/scripts/simplebench/bench_block_job.py > +++ b/scripts/simplebench/bench_block_job.py > @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): > return {'error': 'qemu failed: ' + str(vm.get_log())} > > try: > + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True, > + check=True) > res = vm.qmp(cmd, **cmd_args) > if res != {'return': {}}: > vm.shutdown() > Worth adding a conditional to allow "hot" or "cold" runs? nah?
05.03.2021 04:30, John Snow wrote: > On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote: >> It probably may improve reliability of results when testing in cached >> mode. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> scripts/simplebench/bench_block_job.py | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py >> index 4f03c12169..fa45ad2655 100755 >> --- a/scripts/simplebench/bench_block_job.py >> +++ b/scripts/simplebench/bench_block_job.py >> @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): >> return {'error': 'qemu failed: ' + str(vm.get_log())} >> try: >> + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True, >> + check=True) >> res = vm.qmp(cmd, **cmd_args) >> if res != {'return': {}}: >> vm.shutdown() >> > > Worth adding a conditional to allow "hot" or "cold" runs? nah? > You mean, make this addition optional? Make sense
On 3/5/21 4:11 AM, Vladimir Sementsov-Ogievskiy wrote: > 05.03.2021 04:30, John Snow wrote: >> On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote: >>> It probably may improve reliability of results when testing in cached >>> mode. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> --- >>> scripts/simplebench/bench_block_job.py | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/scripts/simplebench/bench_block_job.py >>> b/scripts/simplebench/bench_block_job.py >>> index 4f03c12169..fa45ad2655 100755 >>> --- a/scripts/simplebench/bench_block_job.py >>> +++ b/scripts/simplebench/bench_block_job.py >>> @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): >>> return {'error': 'qemu failed: ' + str(vm.get_log())} >>> try: >>> + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', >>> shell=True, >>> + check=True) >>> res = vm.qmp(cmd, **cmd_args) >>> if res != {'return': {}}: >>> vm.shutdown() >>> >> >> Worth adding a conditional to allow "hot" or "cold" runs? nah? >> > > You mean, make this addition optional? Make sense > > I was thinking (along the lines of allowing both old and new behavior, in case anyone except you used these scripts) of this sort of thing: def bench_block_job(cmd, cmd_args, qemu_args, drop_cache=True): ... I don't insist on it; I was just earnestly wondering if it had any utility. If it doesn't, don't respin on my account. --js
05.03.2021 19:30, John Snow wrote: > On 3/5/21 4:11 AM, Vladimir Sementsov-Ogievskiy wrote: >> 05.03.2021 04:30, John Snow wrote: >>> On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote: >>>> It probably may improve reliability of results when testing in cached >>>> mode. >>>> >>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>>> --- >>>> scripts/simplebench/bench_block_job.py | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py >>>> index 4f03c12169..fa45ad2655 100755 >>>> --- a/scripts/simplebench/bench_block_job.py >>>> +++ b/scripts/simplebench/bench_block_job.py >>>> @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): >>>> return {'error': 'qemu failed: ' + str(vm.get_log())} >>>> try: >>>> + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True, >>>> + check=True) >>>> res = vm.qmp(cmd, **cmd_args) >>>> if res != {'return': {}}: >>>> vm.shutdown() >>>> >>> >>> Worth adding a conditional to allow "hot" or "cold" runs? nah? >>> >> >> You mean, make this addition optional? Make sense >> >> > > I was thinking (along the lines of allowing both old and new behavior, in case anyone except you used these scripts) of this sort of thing: > > def bench_block_job(cmd, cmd_args, qemu_args, drop_cache=True): ... > > I don't insist on it; I was just earnestly wondering if it had any utility. If it doesn't, don't respin on my account. > Ok, thanks a lot for reviewing! Still, I think, I'll resend
On 3/5/21 11:50 AM, Vladimir Sementsov-Ogievskiy wrote: > 05.03.2021 19:30, John Snow wrote: >> On 3/5/21 4:11 AM, Vladimir Sementsov-Ogievskiy wrote: >>> 05.03.2021 04:30, John Snow wrote: >>>> On 3/4/21 5:17 AM, Vladimir Sementsov-Ogievskiy wrote: >>>>> It probably may improve reliability of results when testing in cached >>>>> mode. >>>>> >>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>>>> --- >>>>> scripts/simplebench/bench_block_job.py | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/scripts/simplebench/bench_block_job.py >>>>> b/scripts/simplebench/bench_block_job.py >>>>> index 4f03c12169..fa45ad2655 100755 >>>>> --- a/scripts/simplebench/bench_block_job.py >>>>> +++ b/scripts/simplebench/bench_block_job.py >>>>> @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): >>>>> return {'error': 'qemu failed: ' + str(vm.get_log())} >>>>> try: >>>>> + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', >>>>> shell=True, >>>>> + check=True) >>>>> res = vm.qmp(cmd, **cmd_args) >>>>> if res != {'return': {}}: >>>>> vm.shutdown() >>>>> >>>> >>>> Worth adding a conditional to allow "hot" or "cold" runs? nah? >>>> >>> >>> You mean, make this addition optional? Make sense >>> >>> >> >> I was thinking (along the lines of allowing both old and new behavior, >> in case anyone except you used these scripts) of this sort of thing: >> >> def bench_block_job(cmd, cmd_args, qemu_args, drop_cache=True): ... >> >> I don't insist on it; I was just earnestly wondering if it had any >> utility. If it doesn't, don't respin on my account. >> > > Ok, thanks a lot for reviewing! Still, I think, I'll resend > Thanks for sharing your benchmarking scripts :) --js
diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py index 4f03c12169..fa45ad2655 100755 --- a/scripts/simplebench/bench_block_job.py +++ b/scripts/simplebench/bench_block_job.py @@ -53,6 +53,8 @@ def bench_block_job(cmd, cmd_args, qemu_args): return {'error': 'qemu failed: ' + str(vm.get_log())} try: + subprocess.run('sync; echo 3 > /proc/sys/vm/drop_caches', shell=True, + check=True) res = vm.qmp(cmd, **cmd_args) if res != {'return': {}}: vm.shutdown()
It probably may improve reliability of results when testing in cached mode. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- scripts/simplebench/bench_block_job.py | 2 ++ 1 file changed, 2 insertions(+)