Re: Performance improvement with 81f730d4

[Lukáš Doktor]

Hello Kevin

Dne 19. 04. 23 v 10:16 Kevin Wolf napsal(a):
> Hi Lukáš,
> 
> Am 19.04.2023 um 08:02 hat Lukáš Doktor geschrieben:
>> Hello Paolo,
>>
>> the perf-ci detected and bisected the 81f730d4 - block, block-backend:
>> write some hot coroutine wrappers by hand - as a performance
>> improvement with 4K read/writes across various profiles. Without
>> pinning it was about 10%, with pinned qemu about 15%.
>>
>>     
>> https://ldoktor.github.io/tmp/RedHat-virtlab722/v8.0.0/130-improvement.html
> 
> I must admit that I don't really understand how to read this page, and
> which build corresponds to which QEMU revision (or even which one is
> older and which one is newer).
> 
> All the build names are links, but they only result in a 404.

The report is originally designed for CI, where the links point to Jenkins 
results. In bisection I'm reusing the same report, replacing the links with 
full qemu commit hashes, which for some reason did not happen in this report. 
I'll take a look at that. The only way to tell the builds apart now is the 
3-letter sha which is used as the build name. The builds are ordered as they 
appear in the git history.

You can see that 81f is the first one that performs differently. I'm using 2 
out of 3 mode to improve reproducibility, which is why there are usually 2 
builds of the same short name.

Usually I attach the bisect log but this time it was so obvious I skipped it. 
Let me attach it now to simplify mapping 3-letter short names to full qemu shas:

git bisect start
# good: [6c50845a9183610cfd4cfffd48dfc704cd340882] hw/i2c/allwinner-i2c: Fix 
subclassing of TYPE_AW_I2C_SUN6I
git bisect good 6c50845a9183610cfd4cfffd48dfc704cd340882
# bad: [7dbd6f8a27e30fe14adb3d5869097cddf24038d6] Update version for v8.0.0-rc4 
release
git bisect bad 7dbd6f8a27e30fe14adb3d5869097cddf24038d6
# bad: [3fe64abcde55cf6f4ea5883106301baad219a7cc] block/nfs: do not poll within 
a coroutine
git bisect bad 3fe64abcde55cf6f4ea5883106301baad219a7cc
# good: [8c6f27e7d85a794698eb1cd32c58df28cece50d1] block: remove 
has_variable_length from BlockDriver
git bisect good 8c6f27e7d85a794698eb1cd32c58df28cece50d1
# good: [9ed98cae151368cc89c4bb77c9f325f7185e8f09] block-backend: ignore 
inserted state in blk_co_nb_sectors
git bisect good 9ed98cae151368cc89c4bb77c9f325f7185e8f09
# bad: [abb02ce0e76a8e00026699a863ab2d11d88f56d4] Merge tag 'for-upstream' of 
https://repo.or.cz/qemu/kevin into staging
git bisect bad abb02ce0e76a8e00026699a863ab2d11d88f56d4
# bad: [81f730d4d0e8af9c0211c3fedf406df0046341a9] block, block-backend: write 
some hot coroutine wrappers by hand
git bisect bad 81f730d4d0e8af9c0211c3fedf406df0046341a9
# first bad commit: [81f730d4d0e8af9c0211c3fedf406df0046341a9] block, 
block-backend: write some hot coroutine wrappers by hand

> 
>> Based on the commit message I guess it's expected, so my question is
>> whether I should report such obvious improvements or only report
>> improvements/regressions that are not that obvious?
> 
> I think it's useful to know that we did indeed fix the performance
> problem, so as far as I am concerned, please do report them.
> 

OK, I'll keep doing that.

> When we're addressing a regression, the most interesting part would of
> course be comparing to the version before the regression was introduced
> (pre-d8fbf9aa85a in this case) to see if the regression was fully fixed.
> Maybe the page you linked above does already explain this and I just
> don't know where to look? :-)
> 

I wasn't aware of this to be addressing d8fbf9aa85a regression. If I find 
enough time I'll try to compare those.

Regards,
Lukáš

> Kevin

OpenPGP_0x26B362E47FCF22C1.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature