Re: [PATCH 0/7] [python] API for macros

[Phil Muldoon <pmuldoon at redhat dot com>]

matt rice <ratmice@gmail.com> writes:

> take 2 on the python macro API...
> In addition to the testsuite, I tested with the last release of
> gcc following: http://gcc.gnu.org/wiki/DebuggingGCC
>
> using variations of the do_stuff function in following script...
> I didn't save exact timing #'s but it was something like
>  2 mins to count all 600+ million macros
>  5 mins to filter all by name with macro.name().startswith()
>  15 mins to call the str() method (which calls all other methods).

I'm more concerned about having 600 million Python objects lying around
indefinitely ;)  (When a macro is invalidated, the Python object still
has to exist so that the user can still call the APIs (to figure out,
hey this macro is now invalid.))

But I looked at your code, and to be honest, this area of GDB is brand
new to me, so I don't feel very qualified to review it.  I will try.  My
main overall concern is removing the macro cache.  Given that we cannot
count the numbers before your change (well not easily, as there is no
real way to script them), I'm a little bit concerned if the above
numbers are significantly impacted by the removal of the macro cache.

> I'd tried doing a deep copy in the gdb.Macro class,
> to avoid all the objfile/obstack/bcache horse pucky evident in this series,
> but i killed it before it completed when working with gcc...

Given that macros can be extremely prevalent in some projects, I think a
deep copy would not be the way to proceed anyway.

> it's not really timing equivalent to that last 15 minutes case since
> we use lots more memory having a deep copy of all the macros in a symtab in a
> list.  Where the 15 minute version does a deep copy, with only one macro's
> contents in memory at a time.
>
> thus, I think it is useful even for large projects (if used with care.)
> this will fall over if someone has has way too many
> macros in a single symtab.  should that happen we can add a macro_map()
> that behaves sort of like the python map function.

We should add it now, IMO, instead of waiting for it to fail later.  I'm
not sure of the exact requirements for the number of macros in symtab to
qualify for this case, but given how widely used GDB is, it will fail,
sooner or later.


> I think a list is the most straight forward approach for general usage,
> and has been shown to work even with projects that use macros extensively.

You did not note your machine specifications, btw.

> With regards to the hash/compare methods, the implementation of those
> is up for debate, I see at least 3 valid ways to compare them and have only one
> comparison function.  right now I have it compare deeply e.g. including the whole include_trail
> other options that seem valid, compare the name,args,defininition, 
> and compare the name,args,definition and partial include_trail
> (just definition location) since they pretty much are equal before expansion,
> and expansion is an 'expression' thing.

I'd rather correct hash uniqueness over comparable performance here.

> There are some implementation quirks described in the documentation,
> some of these are so in the future we can add a gdb.UserMacro which does
> a deep-copy on initialization, I wasn't going to add that unless someone
> requests it.  Python doesn't seem to have any form of java's `interface',
> or abstract base classing at least within our version range,
> we could also hack up something ugly in the future to avoid this documentation,
> (read union'ify the class implementation,
>  or make gdb.Macro have a pointer to a gdb.ObjfileMacro
>  and do the lambda macro: macro.method() inside gdb.Macro)
> I'd personally just leave it there to give us future choice on the matter.
> we can always remove that from the docs if we implement it in a way


I really think we ought to do this now, not what you wanted to hear, I
know.  But I think it would be genuinely useful.  No hacks, though! ;)

Cheers,

Phil