[Libguestfs] [nbdkit PATCH 1/2] sh, eval: Cache .can_zero and .can_flush

[Eric Blake]

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 at 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