Libguestfs - Nov 2015 - Re: [PATCH] tests/c-api: cache available features

Build a list of all the features used in action tests, and lazily read
them as needed.  This reduces the number of guestfs_feature_available
calls for a full run from 117 to 18.
---
 generator/tests_c_api.ml | 77 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/generator/tests_c_api.ml b/generator/tests_c_api.ml
index 6be753f..8c4e5ef 100644
--- a/generator/tests_c_api.ml
+++ b/generator/tests_c_api.ml
@@ -28,6 +28,8 @@ open Optgroups
 open Actions
 open Structs
 
+module StringSet = Set.Make (String)
+
 (* Generate the C API tests. *)
 let rec generate_c_api_tests ()    generate_header CStyle GPLv2plus;
@@ -41,6 +43,7 @@ let rec generate_c_api_tests ()  #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <stdbool.h>
 
 #include \"guestfs.h\"
 #include \"guestfs-internal-frontend.h\"
@@ -53,6 +56,74 @@ let rec generate_c_api_tests ()  #error Missing
GUESTFS_ISO_SYSTEM_ID for the current OS
 #endif
 
+struct feature {
+  const char *name;
+  bool read;
+  bool available;
+};
+
+";
+
+  (* Get a list of all the features. *)
+  let features +    List.fold_left (
+      fun acc { optional = optional; tests = tests } ->
+        let acc +          match optional with
+          | Some group -> StringSet.add group acc
+          | None -> acc in
+        List.fold_left (
+          fun acc test ->
+            match test with
+            | (_, IfAvailable group, _, _) -> StringSet.add group acc
+            | (_, (Always|IfNotCrossAppliance|Disabled), _, _) -> acc
+        ) acc tests
+    ) StringSet.empty all_functions in
+  let features = List.sort compare (StringSet.elements features) in
+  let nr_features = List.length features in
+  pr "size_t nr_features = %d;\n" nr_features;
+  pr "\n";
+  pr "struct feature features[%d] = {\n" nr_features;
+  List.iter (
+    fun feature ->
+      pr "  { .name = \"%s\", .read = false, .available = false
},\n"
+        feature
+  ) features;
+  pr "};\n";
+  pr "\n";
+
+  pr "\
+static bool
+is_feature_available (guestfs_h *g, const char *feature)
+{
+  size_t i;
+
+  for (i = 0; i < nr_features; ++i) {
+    struct feature *f = &features[i];
+
+    if (STRNEQ (f->name, feature))
+      continue;
+
+    if (!f->read) {
+      const char *array[] = { f->name, NULL };
+      int res = guestfs_feature_available (g, (char **) array);
+      if (res < 0) {
+        fprintf (stderr,
+                 \"call to guestfs_feature_available(%%s) failed: %%d,
%%s\\n\",
+                 f->name, guestfs_last_errno (g), guestfs_last_error (g));
+        exit (EXIT_FAILURE);
+      }
+
+      f->available = res > 0;
+      f->read = true;
+    }
+
+    return f->available;
+  }
+
+  return false;
+}
+
 ";
 
   (* Generate a list of commands which are not tested anywhere. *)
@@ -140,12 +211,10 @@ static int
    * support is available in the daemon.
    *)
   let group_test group -    let sym = gensym "features" in
-    pr "  const char *%s[] = { \"%s\", NULL };\n" sym
group;
-    pr "  if (!guestfs_feature_available (g, (char **) %s)) {\n" sym;
+    pr "  if (!is_feature_available (g, \"%s\")) {\n"
group;
     pr "    skipped (\"%s\", \"group %%s not available in
daemon\",\n"
       test_name;
-    pr "             %s[0]);\n" sym;
+    pr "             \"%s\");\n" group;
     pr "    return 0;\n";
     pr "  }\n";
     pr "\n"
-- 
2.1.0

On Wed, Nov 04, 2015 at 06:42:13PM +0100, Pino Toscano
wrote:
> Build a list of all the features used in action tests, and lazily read
> them as needed.  This reduces the number of guestfs_feature_available
> calls for a full run from 117 to 18.
ACK.

But even better if you turned guestfs_feature_available into a
non_daemon_function, added a new guestfs_internal_feature_available
daemon_function, and then cached the results on the library side.
They can never change for the duration of a libguestfs library load,
as far as I know.

That would benefit all libguestfs users.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Wednesday 04 November 2015 20:55:13 Richard W.M. Jones
wrote:
> On Wed, Nov 04, 2015 at 06:42:13PM +0100, Pino Toscano wrote:
> > Build a list of all the features used in action tests, and lazily read
> > them as needed.  This reduces the number of guestfs_feature_available
> > calls for a full run from 117 to 18.
> 
> ACK.
> 
> But even better if you turned guestfs_feature_available into a
> non_daemon_function, added a new guestfs_internal_feature_available
> daemon_function, and then cached the results on the library side.
> They can never change for the duration of a libguestfs library load,
> as far as I know.
> 
> That would benefit all libguestfs users.
You are right indeed, that will be better; patches for that coming up
in a minute.

Please discard this patch.

-- 
Pino Toscano