Re: entry vs pending_entry in dwarf_output::copier

[Roland McGrath <roland at redhat dot com>]

[The list suppressed your post because you didn't use your canonical address.]

> Yes, things are more clear now. Thanks. And dropping the pending_entry
> asap to safe memory also makes sense. But is the state held separately
> in pending_entry really that much and/or can't it just be derived from
> entry itself? 

Well, the proof is in the pudding, so look at the size of the structs to
decide.  I think the various STL containers used there have some nontrivial
sizes, and more indirect memory use when they're nonempty.

> It is somewhat hard to change the hairy bookkeeping without understanding
> how it currently works.

Well, I only kind of remember how it is supposed to work--and it doesn't
really quite work, which is why we're here.  We'll have to muddle through
together.

> The reason I am looking at this is indeed to see if it can be simplified.
> If I am looking at this right then if we move the dwarf_output
> attribute_map held in pending_entry to entry, and we derive
> the children from the identities held in the _m_entries map, then it seems
> we have all needed state. But I might be missing some of the other purposes
> of pending_entry like the usage in pending_dwarf comparator. See below.

You can't go from identity to anything else directly.  You'd have to keep
an iterator stored too, aka <input>::debug_info_entry::const_pointer.
There's also the question of how efficient it is to re-walk and the input
DIEs, i.e. a dwarf_child call plus an _m_entries lookup rather than just
indexing a std::vector.

The attributes map is a bunch of memory use.  So you certainly don't want
that around when you've finalized.  You could make that alone a separate
new/delete'd pointer.  But I'm not really clear on how that is simpler than
having the map live in pending_entry and new/delete on that.

> As far as I understand the circular references are constructed to be
> used in the pending_dwarf comparison. 

To get circular references finalized in the collector at all, you have to
do some monkey business.  There is no "write once" way to copy a circular
list, you always have to back-patch the circularity.

> Again, I am hoping we can get rid of
> that and do the comparison on the original input die tree when we need to
> compare two entries (since we do/should have the references to the original
> tree for both and if attributes are similar in the original trees, they
> should be equal in the newly constructed dwarf_output tree), instead of
> doing them on the pending tree (and again I might be missing why that is
> impossible).

Comparing against the originals multiple times is much more costly for the
attribute values.  Consider a string attribute.  We take the input string
once, hash and uniquify it in the collector, and then just keep the pointer
(const value_string *) to that.  Thereafter we can do comparisons with
simple pointer equality.  Whereas going back to the input, we have to do
real string comparisons again.  At least that's my instinct about the costs.
Everything bears checking with measurement, of course.


Thanks,
Roland