Re: [PATCH] cirrus: don't run full qtest on macOS

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 06/01/2021 13:36, Daniel P. Berrangé wrote:

>> The Cirrus CI macOS build hosts have exhibited a serious performance
>> degradation in recent months. For example the "qom-test" qtest takes
>> over an hour for only the qemu-system-aarch64 binary. This is as much
>> 20-40 times slower than other environments. The other qtests all show
>> similar performance degradation, as do many of the unit tests.
>>
>> This does not appear related to QEMU code changes, since older git
>> commits which were known to fully complete in less than 1 hour on
>> Cirrus CI now also show similar bad performance. Either Cirrus CI
>> performance has degraded, or an change in its environment has exposed
>> a latent bug widely affecting all of QEMU. Debugging the qom-test
>> showed no easily identified large bottleneck - every step of the
>> test execution was simply slower.
> 
> It appears I might be mistaken here. On IRC it was reported that
> going back furrther to v5.1.0 shows good performance in Cirrus
> still.
> 
> I had only gone back as far as 2a5a79d1b57280edd72193f6031de3feb682154e
> which I thought was fast originally.
> 
> So somewhere between v5.1.0 and 2a5a79 we apparently regressed.

I tested a few macos cirrus-ci builds after the meson conversion and found that they 
were working fine, so whatever is affecting the macos build must be related to a QEMU 
change.

A full bisect proved to be too tricky due to the instability of the tree at that 
point in time, however reading through "git log" and attempting some builds at merges 
I thought might be related I discovered that the slowness was introduced by this PR:


commit b7092cda1b36ce687e65ab1831346f9529b781b8
Merge: 497d415d76 eb94b81a94
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Fri Oct 9 13:20:46 2020 +0100

     Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2020-10-09' into 
staging

     Monitor patches for 2020-10-09

     # gpg: Signature made Fri 09 Oct 2020 06:16:51 BST
     # gpg:                using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653
     # gpg:                issuer "armbru@redhat.com"
     # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
     # gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>" [full]
     # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

     * remotes/armbru/tags/pull-monitor-2020-10-09:
       block: Convert 'block_resize' to coroutine
       block: Add bdrv_lock()/unlock()
       block: Add bdrv_co_enter()/leave()
       util/async: Add aio_co_reschedule_self()
       hmp: Add support for coroutine command handlers
       qmp: Move dispatcher to a coroutine
       qapi: Add a 'coroutine' flag for commands
       monitor: Make current monitor a per-coroutine property
       qmp: Call monitor_set_cur() only in qmp_dispatch()
       qmp: Assert that no other monitor is active
       hmp: Update current monitor only in handle_hmp_command()
       monitor: Use getter/setter functions for cur_mon
       monitor: Add Monitor parameter to monitor_get_cpu_index()
       monitor: Add Monitor parameter to monitor_set_cpu()

     Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Fortunately that PR could be bisected and that led me this commit:


9ce44e2ce267caf5559904a201aa1986b0a8326b is the first bad commit
commit 9ce44e2ce267caf5559904a201aa1986b0a8326b
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Mon Oct 5 17:58:50 2020 +0200

     qmp: Move dispatcher to a coroutine

     This moves the QMP dispatcher to a coroutine and runs all QMP command
     handlers that declare 'coroutine': true in coroutine context so they
     can avoid blocking the main loop while doing I/O or waiting for other
     events.

     For commands that are not declared safe to run in a coroutine, the
     dispatcher drops out of coroutine context by calling the QMP command
     handler from a bottom half.

     Signed-off-by: Kevin Wolf <kwolf@redhat.com>
     Reviewed-by: Markus Armbruster <armbru@redhat.com>
     Message-Id: <20201005155855.256490-10-kwolf@redhat.com>
     Reviewed-by: Markus Armbruster <armbru@redhat.com>
     Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
     Signed-off-by: Markus Armbruster <armbru@redhat.com>


Given that Peter can run the tests manually, I'm not exactly sure why the cirrus-ci 
environment is different - all I can think of is that it could be related to running 
in a headless terminal.

For reference running cirrus-ci on the last good commit 04f22362f1 "qapi: Add a 
'coroutine' flag for commands" gave me a total runtime of 35 mins.


ATB,

Mark.