Re: module cleanup [3/n]

[Gary V. Vaughan]

Hi Eric,

On Sep 7, 2007, at 11:38 PM, Eric Blake wrote:
> According to Eric Blake on 9/6/2007 4:56 PM:
> > - Module ref-counting is screwy.  The current unload code assumes  
> > that builtins
> > associated with a module must only be unloaded once the module  
> > refcount is at
> > one and will be dropping to 0.  But that's a bit presumptuous,  
> > since the
> > libltdl ref-count could include 3rd party loads of the same module  
> > that should
> > have no impact to our symbol table.  Also, we weren't decreasing  
> > the ref-count
> > of resident modules, which meant that symbols from such a module  
> > weren't always
> > being removed from the symbol table at the right time
> >
> > - Speaking of refcounts, the m4modules builtin only shows a module  
> > once, even
> > if it has been loaded multiple times.  Maybe it is worth adding  
> > another builtin
> > to the load module that can query the current refcount of a given  
> > module name?
>
> This adds a refcount builtin to the load module.  I still think it  
> is a
> bit odd that importing helper symbols from an m4 library affects the
> symbol table, but didn't see an easy way to fix it as part of this  
> patch.

I'm not sure I understand you.  Could you explain what you mean by  
'helper symbol',
and what 'symbol table' you are referring to?

> $ m4 -m load
> refcount(`m4')
> 1
> unload(`m4')refcount(`m4') dnl no longer in table
> 0 dnl no longer in table
>
> esyscmd(`echo')
>
>
> refcount(`m4') dnl esyscmd imports from m4 module, reloading macros
> 1

I see no reason to add another arbitrary builtin, where m4modules  
itself can
easily be extended to provide sufficient functionality.  Let's revert  
this, and
instead have m4modules list the stack of loaded modules.

> > - If we add symbol caching, then it becomes trivial to store  
> > information
> > associated with a builtin (back to my original thought of adding  
> > the ability to
> > trace a builtin regardless of the name it is invoked by); without  
> > a cache,
> > there is no convenient place to do per-builtin tracking since we  
> > can't
> > necessarily write into the memory owned by a module
>
> Not quite symbol caching, but I did speed up the lt_dlinterface_id  
> filter.
> The idea is that if we have ever associated a non-NULL m4_module*  
> with a
> given lt_dlhandle, then we have previously determined that the handle
> meets our needs, so we need not waste time with further lt_dlsym  
> calls to
> check that point.

[[snip]]

> Index: m4/m4private.h
> ===================================================================
> RCS file: /sources/m4/m4/m4/m4private.h,v
> retrieving revision 1.83
> diff -u -p -r1.83 m4private.h
> --- m4/m4private.h      6 Sep 2007 22:58:26 -0000       1.83
> +++ m4/m4private.h      7 Sep 2007 22:32:35 -0000
> @@ -147,6 +147,7 @@ struct m4 {
>  struct m4_module
>  {
>    lt_dlhandle  handle;         /* ltdl module information.  */
> +  int          refcount;       /* count of loads not matched by unload.  */
>    /* TODO: add struct members, such as copy of builtins (so that we
>       can store additional information about builtins, and so that the
>       list isn't changed by the module behind our backs once we have
> @@ -161,6 +162,11 @@ extern void            m4__module_exit (m4 *con
>  extern m4_module *  m4__module_next (m4_module *);
>  extern m4_module *  m4__module_find (const char *name);
>
> +/* Fast macro versions of symbol table accessor functions, that also
> +   have an identically named function exported in m4module.h.  */
> +#ifdef NDEBUG
> +# define m4_module_refcount(M) ((M)->refcount)
> +#endif

If we were to keep the refcount builting, libltdl tracks it for us,  
so there is
no need to duplicate that code again. The libltdl refcount is  
retrieved with:

    lt_dlgetinfo(module->handle)->ref_count

Cheers,
        Gary
--
  ())_.              Email me: address@hidden
  ( '/           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