Re: [patch] Speed up find_pc_section

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 22 July 2009 18:16:06, Paul Pluzhnikov wrote:
> On Wed, Jul 22, 2009 at 9:32 AM, Pedro Alves<pedro@codesourcery.com> wrote:
> 
> > In the OBJF_USERLOADED case, you rebuild the map when you don't
> > really need to.
> 
> I think it's pretty clear that what I really need is a new 'remove_objfile'
> kind of observer.

Would setting objfiles_updated_p from e.g., unlink_objfile work?
Both the map and unlink_objfile/free_objfile are private/implemented in
objfiles.c; if nothing else outside needs the observer, we can avoid
adding it.  Otherwise, a new observer is fine.  (the more transparent
the cache is, the better the design looks, IMHO)

> > You're also likely breaking xcoffsolib.c and the vmap code in exec.c, as
> > it calls free_objfile without going through observers.
> 
> This would be handled by the 'remove_objfile' observer, I think.

I think so.

> 
> > Given the new_objfile observer, do we still need the exec_changed and
> > solib observers?
> 
> No, I've removed that (new patch attached).

I think that the need for the solib load/unload observer
would go away too if the map is flushed on objfile
removal/freeing/unlinking as well?  Why would you need to know about
solibs, if without the cache (that is, before the original patch)
we'd just iterate over the objfiles' sections?  That is, the lifetime
of the cache should be at most the same as of the objfiles'
sections, yes?

Come to look at it deeper, what is happening with
symbol_file_add_with_addrs_or_offsets, if the objfile has only
minimal symbols (but still has sections)?  allocate_objfile is called,
which builds the section table, but, there's a path that does an
early return before calling the new_objfile observer, if there are
no symbols.

> > Please look around and check that objfile->section_offsets or
> > obj_section_offset isn't being used as an lvalue somewhere where
> > the map cache isn't being flushed.
> 
> I think new_objfile/remove_objfile will take care of any of these.

Okay then.

> Should I revert the find_pc_section patch, work up remove_objfile observer
> patch, then re-apply find_pc_section patch once it's in better shape?

It would be cleaner and easier to review, easier for you (I think),
and better for the archives in the future, IMHO.  But I don't mind
much if a new patch is cooked on top.

-- 
Pedro Alves