[PATCH 0/4] qemuxml2*test: Significantly improve performance by caching qemu capabilities and qapi schema
This results in 1/3 of the original execution time. Peter Krempa (4): qemuxml2argvtest: Cache QAPI schema between tests testCompareXMLToArgvValidateSchema: Improve and fix helper for testing everything testQemuInfoSetArgs: Use curly braces in else section qemu*xml2*test: Cache capabilities between tests tests/qemustatusxml2xmltest.c | 3 ++- tests/qemuxml2argvtest.c | 26 +++++++++++++++++++------- tests/qemuxml2xmltest.c | 3 ++- tests/testutilsqemu.c | 24 +++++++++++++++++++----- tests/testutilsqemu.h | 2 ++ 5 files changed, 44 insertions(+), 14 deletions(-) -- 2.29.2
It's quite wasteful to reparse the QAPI schema for each _CAPS_ test. Add a simple cache filled lazily by encountered schemas. The time saving on my box is quite significant: real 0m3.318s user 0m3.203s sys 0m0.107s vs real 0m2.223s user 0m2.134s sys 0m0.084s Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvtest.c | 17 ++++++++++++++--- tests/testutilsqemu.h | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index db438c5466..647187404c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -523,13 +523,22 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, qemuDomainObjPrivatePtr priv = NULL; size_t nargs = 0; size_t i; - g_autoptr(GHashTable) schema = NULL; + GHashTable *schema = NULL; g_autoptr(virCommand) cmd = NULL; unsigned int parseFlags = info->parseFlags; bool netdevQAPIfied = false; - if (info->schemafile) - schema = testQEMUSchemaLoad(info->schemafile); + if (info->schemafile) { + /* lookup and insert into cache if not found */ + if (!g_hash_table_lookup_extended(info->qapiSchemaCache, + info->schemafile, + NULL, (void **) &schema)) { + schema = testQEMUSchemaLoad(info->schemafile); + g_hash_table_insert(info->qapiSchemaCache, + g_strdup(info->schemafile), + schema); + } + } /* comment out with line comment to enable schema checking for non _CAPS tests if (!schema) @@ -779,6 +788,7 @@ mymain(void) int ret = 0; g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = NULL; + g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -872,6 +882,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ + info.qapiSchemaCache = qapiSchemaCache; \ if (testQemuInfoSetArgs(&info, capslatest, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 0c6f0ea378..86ba69e96b 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -66,6 +66,7 @@ struct testQemuInfo { unsigned int parseFlags; virArch arch; char *schemafile; + GHashTable *qapiSchemaCache; }; virCapsPtr testQemuCapsInit(void); -- 2.29.2
The schema validator has a comment which allows checking all xml2argv input files for schema validity by forcing the latest schema onto files which don't have any schema. Fix it so that it works properly with the caching introduced in previous commit. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvtest.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 647187404c..2d538bee9c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -528,6 +528,11 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, unsigned int parseFlags = info->parseFlags; bool netdevQAPIfied = false; + /* comment out with line comment to enable schema checking for non _CAPS tests + if (!info->schemafile) + info->schemafile = testQemuGetLatestCapsForArch(virArchToString(info->arch), "replies"); + // */ + if (info->schemafile) { /* lookup and insert into cache if not found */ if (!g_hash_table_lookup_extended(info->qapiSchemaCache, @@ -540,11 +545,6 @@ testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv, } } - /* comment out with line comment to enable schema checking for non _CAPS tests - if (!schema) - schema = testQEMUSchemaLoadLatest(virArchToString(info->arch)); - // */ - if (!schema) return 0; -- 2.29.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index c2cf3be9ac..a96c9d487a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -779,8 +779,10 @@ testQemuInfoSetArgs(struct testQemuInfo *info, if (STREQ(capsver, "latest")) { capsfile = g_strdup(virHashLookup(capslatest, capsarch)); stripmachinealiases = true; - } else capsfile = g_strdup_printf("%s/caps_%s.%s.xml", - TEST_QEMU_CAPS_PATH, capsver, capsarch); + } else { + capsfile = g_strdup_printf("%s/caps_%s.%s.xml", + TEST_QEMU_CAPS_PATH, capsver, capsarch); + } if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) goto cleanup; -- 2.29.2
Invoking the XML parser every time is quite expensive. Since we have a deep copy function for 'virQEMUCapsPtr' object, we can cache the parsed results lazily. This brings significant speedup to qemuxml2argvtest: real 0m2.234s user 0m2.140s sys 0m0.089s vs. real 0m1.161s user 0m1.087s sys 0m0.072s qemuxml2xmltest benefits too: real 0m0.879s user 0m0.801s sys 0m0.071s vs. real 0m0.466s user 0m0.424s sys 0m0.040s Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemustatusxml2xmltest.c | 3 ++- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 3 ++- tests/testutilsqemu.c | 18 +++++++++++++++--- tests/testutilsqemu.h | 1 + 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c index 67a070c986..2b2c409cba 100644 --- a/tests/qemustatusxml2xmltest.c +++ b/tests/qemustatusxml2xmltest.c @@ -73,6 +73,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; capslatest = testQemuGetLatestCaps(); @@ -109,7 +110,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ ARG_QEMU_CAPS, QEMU_CAPS_LAST, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d538bee9c..d6d707cd24 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -789,6 +789,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = NULL; g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -883,7 +884,7 @@ mymain(void) .name = _name, \ }; \ info.qapiSchemaCache = qapiSchemaCache; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ testInfoSetPaths(&info, _suffix); \ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5cd945f28f..01ac5886f2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -85,6 +85,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL; capslatest = testQemuGetLatestCaps(); @@ -130,7 +131,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ __VA_ARGS__, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index a96c9d487a..1dcf5caf6a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -681,6 +681,7 @@ testQemuCapsIterate(const char *suffix, int testQemuInfoSetArgs(struct testQemuInfo *info, + GHashTable *capscache, GHashTable *capslatest, ...) { va_list argptr; @@ -773,6 +774,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info, if (!qemuCaps && capsarch && capsver) { bool stripmachinealiases = false; + virQEMUCapsPtr cachecaps = NULL; info->arch = virArchFromString(capsarch); @@ -784,11 +786,21 @@ testQemuInfoSetArgs(struct testQemuInfo *info, TEST_QEMU_CAPS_PATH, capsver, capsarch); } - if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) + if (!g_hash_table_lookup_extended(capscache, capsfile, NULL, (void **) &cachecaps)) { + if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) + goto cleanup; + + if (stripmachinealiases) + virQEMUCapsStripMachineAliases(qemuCaps); + + cachecaps = qemuCaps; + + g_hash_table_insert(capscache, g_strdup(capsfile), g_steal_pointer(&qemuCaps)); + } + + if (!(qemuCaps = virQEMUCapsNewCopy(cachecaps))) goto cleanup; - if (stripmachinealiases) - virQEMUCapsStripMachineAliases(qemuCaps); info->flags |= FLAG_REAL_CAPS; /* provide path to the replies file for schema testing */ diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 86ba69e96b..e1b386f234 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -110,6 +110,7 @@ int testQemuCapsIterate(const char *suffix, void *opaque); int testQemuInfoSetArgs(struct testQemuInfo *info, + GHashTable *capscache, GHashTable *capslatest, ...); void testQemuInfoClear(struct testQemuInfo *info); -- 2.29.2
On Fri, 19 Feb 2021 16:53:21 +0100 Peter Krempa <pkrempa@redhat.com> wrote:
Invoking the XML parser every time is quite expensive. Since we have a deep copy function for 'virQEMUCapsPtr' object, we can cache the parsed results lazily.
This brings significant speedup to qemuxml2argvtest:
real 0m2.234s user 0m2.140s sys 0m0.089s
vs.
real 0m1.161s user 0m1.087s sys 0m0.072s
qemuxml2xmltest benefits too:
real 0m0.879s user 0m0.801s sys 0m0.071s
vs.
real 0m0.466s user 0m0.424s sys 0m0.040s
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemustatusxml2xmltest.c | 3 ++- tests/qemuxml2argvtest.c | 3 ++- tests/qemuxml2xmltest.c | 3 ++- tests/testutilsqemu.c | 18 +++++++++++++++--- tests/testutilsqemu.h | 1 + 5 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c index 67a070c986..2b2c409cba 100644 --- a/tests/qemustatusxml2xmltest.c +++ b/tests/qemustatusxml2xmltest.c @@ -73,6 +73,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL;
capslatest = testQemuGetLatestCaps(); @@ -109,7 +110,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ ARG_QEMU_CAPS, QEMU_CAPS_LAST, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d538bee9c..d6d707cd24 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -789,6 +789,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(GHashTable) capslatest = NULL; g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify) virHashFree); + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData);
fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE);
@@ -883,7 +884,7 @@ mymain(void) .name = _name, \ }; \ info.qapiSchemaCache = qapiSchemaCache; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ __VA_ARGS__, ARG_END) < 0) \ return EXIT_FAILURE; \ testInfoSetPaths(&info, _suffix); \ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5cd945f28f..01ac5886f2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -85,6 +85,7 @@ mymain(void) g_autofree char *fakerootdir = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(GHashTable) capslatest = NULL; + g_autoptr(GHashTable) capscache = virHashNew(virObjectFreeHashData); g_autoptr(virConnect) conn = NULL;
capslatest = testQemuGetLatestCaps(); @@ -130,7 +131,7 @@ mymain(void) static struct testQemuInfo info = { \ .name = _name, \ }; \ - if (testQemuInfoSetArgs(&info, capslatest, \ + if (testQemuInfoSetArgs(&info, capscache, capslatest, \ __VA_ARGS__, \ ARG_END) < 0 || \ qemuTestCapsCacheInsert(driver.qemuCapsCache, info.qemuCaps) < 0) { \ diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index a96c9d487a..1dcf5caf6a 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -681,6 +681,7 @@ testQemuCapsIterate(const char *suffix,
int testQemuInfoSetArgs(struct testQemuInfo *info, + GHashTable *capscache, GHashTable *capslatest, ...) { va_list argptr; @@ -773,6 +774,7 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
if (!qemuCaps && capsarch && capsver) { bool stripmachinealiases = false; + virQEMUCapsPtr cachecaps = NULL;
info->arch = virArchFromString(capsarch);
@@ -784,11 +786,21 @@ testQemuInfoSetArgs(struct testQemuInfo *info, TEST_QEMU_CAPS_PATH, capsver, capsarch); }
- if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) + if (!g_hash_table_lookup_extended(capscache, capsfile, NULL, (void **) &cachecaps)) { + if (!(qemuCaps = qemuTestParseCapabilitiesArch(info->arch, capsfile))) + goto cleanup; + + if (stripmachinealiases) + virQEMUCapsStripMachineAliases(qemuCaps); + + cachecaps = qemuCaps; + + g_hash_table_insert(capscache, g_strdup(capsfile), g_steal_pointer(&qemuCaps)); + } + + if (!(qemuCaps = virQEMUCapsNewCopy(cachecaps))) goto cleanup;
- if (stripmachinealiases) - virQEMUCapsStripMachineAliases(qemuCaps); info->flags |= FLAG_REAL_CAPS;
/* provide path to the replies file for schema testing */ diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 86ba69e96b..e1b386f234 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -110,6 +110,7 @@ int testQemuCapsIterate(const char *suffix, void *opaque);
int testQemuInfoSetArgs(struct testQemuInfo *info, + GHashTable *capscache, GHashTable *capslatest, ...); void testQemuInfoClear(struct testQemuInfo *info);
Series looks fine to me. One thing I find a little bit confusing in this patch is that you have both 'cachecaps' and 'capscache' in the same function. I think just renaming 'cachecaps' to 'caps' (or even 'cachedcaps' with a d) would make things slightly less confusing. Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
participants (2)
-
Jonathon Jongsma -
Peter Krempa