Re: [Libguestfs] [nbdkit PATCH 1/2] sh, eval: Cache .can_zero and .can_flush
On 3/18/20 8:21 PM, Eric Blake wrote:
In commit c306fa93ab and neighbors (v1.15.1), a concerted effort
went
into caching the results of .can_FOO callbacks, with commit messages
demonstrating that a plugin with a slow callback should not have that
delay magnified multiple times. But nothing was added to the
testsuite at the time, and with the sh and eval plugins, we still have
two scenarios where we did not take advantage of the nbdkit cache
because we directly called things ourselves (one pre-existing since
v1.13.9, another introduced in v1.15.1 right after the cleanups). Fix
those spots by reworking the handle we pass to nbdkit, then enhance
test-eflags.sh to ensure we don't regress again.
Note that nbdkit does not yet cache .thread_model; that will be
addressed in the next patch. Furthermore, we end up duplicating the
caching that nbdkit itself does, because it would be a layering
violation for us to have enough information to call into
backend_can_flush().
Fixes: 05c5eed6f2
Fixes: 8490694d08
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
@@ -457,8 +473,14 @@ int
sh_can_flush (void *handle)
{
const char *method = "can_flush";
- const char *script = get_script (method);
- return boolean_method (script, method, handle, 0);
+ const char *script;
+ struct sh_handle *h = handle;
+
+ if (h->can_flush >= 0)
+ return h->can_flush;
+
+ script = get_script (method);
+ return h->can_flush = boolean_method (script, method, handle, 0);
}
Thinking about this more, maybe the real problem is that all language
bindings that have to wrap scripts (OCaml and Rust are exceptions as
they directly call into the C code; but lua, perl, python, ruby, tcl,
and sh all fit into the category I'm describing) MUST define .can_FOO at
the C level because they don't know a priori whether the script they
will be loading will also provide a .can_FOO callback, so we have a lot
of duplicated code in each language binding.
Life might be easier if we change the C contract for the .can_FOO
callbacks to support the special return value of -2 to behave the same
as if .can_FOO were missing. In a way, it would make it easier for the
remaining languages to behave more like sh's special MISSING return
value (sh is an oddball because we can't even probe whether .FOO is
missing so .can_FOO is mandatory in the script; but in eval, python,
etc, it's easy to implement a C binding for .can_FOO that introspects
the script). I'll spin up a v2 patch along those lines, so we can
compare the difference.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org