[PATCH 0/4] qemuxml2*test: Significantly improve performance by caching qemu capabilities and qapi schema
9:53 a.m.
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
9:53 a.m.
New subject: [PATCH 1/4] qemuxml2argvtest: Cache QAPI schema between tests
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(a)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
9:53 a.m.
New subject: [PATCH 2/4] testCompareXMLToArgvValidateSchema: Improve and fix helper for testing everything
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(a)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
9:53 a.m.
New subject: [PATCH 3/4] testQemuInfoSetArgs: Use curly braces in else section
Signed-off-by: Peter Krempa <pkrempa(a)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
9:53 a.m.
New subject: [PATCH 4/4] qemu*xml2*test: Cache capabilities between tests
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(a)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
10:49 a.m.
New subject: [PATCH 4/4] qemu*xml2*test: Cache capabilities between tests
On Fri, 19 Feb 2021 16:53:21 +0100
Peter Krempa <pkrempa(a)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(a)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(a)redhat.com>
1372
days inactive
1372
days old
5 comments
2 participants
participants (2)
-
Jonathon Jongsma -
Peter Krempa