Daniel P. Berrangé <berra...@redhat.com> writes: > On Tue, Jul 07, 2020 at 06:45:57AM +0200, Thomas Huth wrote: >> On 27/05/2020 10.47, Markus Armbruster wrote: >> > "info qom-tree" prints children in unstable order. This is a pain >> > when diffing output for different versions to find change. Print it >> > sorted. >> > >> > Signed-off-by: Markus Armbruster <arm...@redhat.com> >> > --- >> > qom/qom-hmp-cmds.c | 24 ++++++++++++++++-------- >> > 1 file changed, 16 insertions(+), 8 deletions(-) >> >> Hi Markus, >> >> this patch causes a slow down of the qtests which becomes quite massive >> when e.g. using the ppc64 and thourough testing. When I'm running >> >> QTEST_QEMU_BINARY="ppc64-softmmu/qemu-system-ppc64" time \ >> ./tests/qtest/device-introspect-test -m slow | tail -n 10 >> >> the test runs for ca. 6m40s here before the patch got applied, and for >> mor than 20 minutes after the patch got applied! > > I think the test case itself could be optimized. Currently it does > approx > > for each device type > info qom-tree > device_addr type,help > info qom-tree > > it compares the before/after qom-tree to look for stray objects or > to try to trigger crashes. > > The info qom-tree calls could be pushed outside the loop > > info qom-tree > for each device type > device_addr type,help > info qom-tree > > Taking /x86_64/device/introspect/concrete/defaults/pc-q35-5.1 as a > example, this change is the difference between 20 seconds running and > 3 seconds running.
Patch? > Reverting Markus' change actually didn't make much difference, only > reducing the 20 seconds to 17 seconds. I found and plugged a memory leak. I haven't checked its impact on test performance. > The downside is that if there is a stray object/crash, it would not > immediately associate with the device type. I'm not sure that's a > real problem though. Especially if we are running this as pre-merge > CI we'll only need to look at the patch series to find the broken > device. If this is quick enough that we can run it as standard, > instead of only with -m slow, then its a net win I think. Easier investigation of failures is a valid argument, but not at this price. Our "make check" is painfully slow.