Re: module cleanup [2/n]

Gary V. Vaughan Thu, 13 Sep 2007 09:35:13 -0700

Hi Eric,

On Sep 13, 2007, at 1:40 PM, Eric Blake wrote:

According to Gary V. Vaughan on 9/11/2007 4:06 PM:
    Wrap lt_dlhandle in struct m4_module.
Wrapping like this introduces an additional dereference every time a module is accessed. For this, and aesthetic reasons, I'd rather see m4_module as
a simple typedef alias.
libltdl can only attach a single pointer to the lt_dlhandle. So it has to be a pointer to a struct for us to store more information than that. I'm still not convinced whether handing m4 clients the raw lt_dlhandle (under
a type-safe opaque alias) is any more efficient than handing them the
pointer to the struct associated with the handle, and having that struct
point back to the handle (which is what I implemented).

It saves a pointer dereference at every call across the libltdl interface boundary. That isn't a big deal if it only happens during module load/unload,

though we need to be careful not to slow down any operations involved in
using the builtins from the loaded modules...

One benefit of my
approach is that we can cache anything in the struct, reducing the need to
continually rely on libltdl accessors to relearn something that hasn't
changed since the last time we checked (for example, the implementation of m4_get_module_name could be sped up by caching lt_dlgetinfo(handle)- >name in the m4_module, at which point a fast accessor macro is easier to write).

Although it adds a stack frame, lt_dlgetinfo merely returns the address of

its own cached values, although it is certainly worth measuring whether
duplicating that cache outside the ltdl layer speeds things up enough to

compensate for the slowdown accrued from the additional pointer dereferences

inherent in using a struct rather than an alias.

As you say, that might all be moot if it turns out we need to store more than one datum address per module (and consequently have to use the struct)...

    (m4_module): New declaration.  Simple for now, but intended for
    growth.
I would say that you have found a deficiency in the libltdl apis if you
need
to duplicate information that is already tracked by libltdl itself, or that
you are unable to store with lt_dlcaller_set_data...
I found that storing the m4_module* using lt_dlcaller_set_data worked all
right, once I fixed the lt_dlcaller_get_data bug.

The benefit of the old approach is that if the only address that needs storing

is the address of the lt_dlhandle itself, we not only save the pointer
dereferences but also the stack frame for lt_dlcaller_get_data.

    (m4_module_makeresident, m4_module_refcount): New functions.


These should be macros (at least with NDEBUG) that defer to the
identical ltdl
calls, to avoid the extra stack frame.


I'll keep that on my list of future cleanups.


Cool!

    (module_close): Delete, and inline into module_remove.
I rather liked the symmetry before this change :-(
This whole area is still a bit awkward - particularly since both functions
were having to strdup the module name in case of error.

You're right that there are still some symmetry issues to work out, as
well as correctly dividing the work between the three layers:
m4_module_import/m4_module_? - opens a module in order to import an entry
point from it (for example, libgnu's esyscmd needs to affect libm4's
sysval builtin) - this currently affects the m4 symbol table, but I'm not
sure that it should, and I don't know if a symmetric partner is needed


Nice analysis :-)

In an ideal world, once we have an import that potentially lt_dlopen's a
module without sucking the builtins into the m4 symbol table, we should
also have an m4_module_unimport to decrement the refcount and possibly
lt_dlclose the module.

m4_module_load/m4_module_unload - opens modules and affects m4 symbol
table, designed for use by libload and other m4 modules

m4__module_open/m4__module_? - opens a module, but leaves m4 symbol table

alone, designed for use by internals, such as src/freeze.c


This is where I'm missing m4__module_close.

static module_?/static module_remove - handles the libltdl handle open and
close, designed for use only by module.c


static module_add?

I'll think about it some more, and try to clean it up better.


Cool!

Cheers,
        Gary
--
  ())_.              Email me: [EMAIL PROTECTED]
  ( '/           Read my blog: http://blog.azazil.net
  / )=         ...and my book: http://sources.redhat.com/autobook
`(_~)_      Join my AGLOCO Network: http://www.agloco.com/r/BBBS7912

PGP.sig
Description: This is a digitally signed message part

_______________________________________________
M4-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/m4-patches

Re: module cleanup [2/n]

Reply via email to