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>