On Tue, 2011-02-01 at 21:19 -0800, Roland McGrath wrote:
> But having all hashes (local and global) in pending_entry would
be
> beneficial. Currently however we try to drop the pending_entry as soon
> as possible whenever we finalize an entry. So maybe we should move them
> both into entry. Also because entry already has the parent which is used
> in the tracker for the matching algorithm.
The die_info_pair is the place that makes sense to me for storing more
information about the final state of things. That is, either in
dwarf_output::debug_info_entry or in die_info. Those live permanently in
the collector. That's where you benefit from having full hash values on
hand to compare, because when you need to do comparisons it's against
existing objects already final in the collector and found there via hashes.
If you had them in copier::entry, how would you ever find them there when
you need them?
Yeah, I finally figured that out too. Bleah.
Sorry for running in the wrong direction.
So all hashers used for putting things into the dwarf_output_collector
are defined on the types we actually put into the collector. Which does
make kind of sense, but does raise the question whether we aren't
creating objects to match against only to immediately throw them away
because they are already in the collector? Although I guess that if this
is really a huge problem we should be happy because there is lots of
sharing. The only reason I bring this up (not to cover up I was looking
in the wrong direction) is that it might be somewhat wasteful. For
example as far as I can see the attributes_type hash is only used to
find the hash bucket when inserting a new attrs_pair. So it would be
nice if we had to only provide the bucket hash number when adding a new
element and not have to store the hash value in the actual element in
that case. Or maybe I am (again) missing something.
die_info also has _m_parent, which is what's used by
copier::tracker.
> Yes, again I admit that the way this evolved was mainly because of the
> way I was most easily able to change the code. Ideally I would have
> added the local hash code to the attributes themselves, but it seemed
> dwarf_data::value_reference, but I really couldn't figure out how things
> got initialized correctly in that case. And it wasn't clear to me
> whether the concept of local hashes really made much sense to any other
> subclass/template instance of that type. I'll make sure the final
> algorithm used for the hash calculation will be documented in the code.
It's certainly possible to do it the other way. You just don't break up
the hash calculation along the lines of the attributes_type/children_type
objects. Instead, do it all at the entry level.
I am not sure if I understand precisely what you mean here. We certainly
need a hash based on the new local hash idea for the attributes_type,
since that is where most of the collisions happen now. Do you mean,
calculate the local hash for/at the entry/die level, and leave it at
that, then use that local hash value in the calculation of the
attributes_type hash?
> Yeah, I actually had wanted to do a trick like that in the
> attributes_type do_hash () function, but I didn't know how to get at the
> _m_value of the attr_value there. Which is also why I ended up using
> what_space to detect whether the attribute was a reference value or not.
You can just use av->_m_value directly. If you can't, then you just
have to add:
friend class dwarf_output::attributes_type;
into the 'class attr_value' definition.
Right, yes that would work. But isn't using friend considered breaking
abstraction? Maybe it is common usage in c++, but it feels very much
like arbitrarily breaking data encapsulation.
> I admit that my c++ typing knowledge fails me again in this
example.
> How can it be that this template variant of reference () returns ...
> while the dwarf_data::attr_value::reference method is defined as
> inline const die_ptr &reference () const with typedef typename
> impl::debug_info_entry::pointer die_ptr; Since impl is dwarf_output, it
> seems somewhat curious that this method is allowed to change the type of
> the return value. Again this kind of breaks my vision of the abstraction
> interface.
You have the wrong idea of how the abstraction binding works. A template
is not like a base class. The various dwarf* classes are "template
compatible". What that means is that you can use the methods of the same
names in the same ways. Put another way, it means the same method in each
of two implementations will have return types that are template-compatible.
I admit I find this very hard to reason about.
So if we have the following:
class dwarf_data
{
template<class impl, typename vw = value<impl> >
class attr_value
{
typedef typename impl::debug_info_entry::pointer die_ptr;
inline die_ptr &reference ()
{
return variant<typename vw::value_reference> ().ref;
}
}
}
class dwarf_output
{
class attr_value
: public dwarf_data::attr_value<dwarf_output, value>
{
}
}
class pending_dwarf
{
struct attr_value
: public dwarf_output::attr_value
{
inline typename debug_info_entry::const_pointer reference () const
{
}
}
}
How do I tell that the return type of reference () is not inherited
through the template base (base) class, but is actually a template value
itself. And what is the return type template value precisely for
reference () in this case?
> No it didn't directly answer my question of "how the
hell do I trick c++
> into doing what I want in this case", but it does show me I should
> concentrate on moving all (local) hashing logic into the (pending) entry
> which can rely on the input die tree and stay as far away from anything
> that could become part of pending_dwarf as I can for now :)
Well, I can't tell if we are really making progress here or not.
But I'll hope.
Well, to be honest, I was making progress, but into the wrong
direction...
Cheers,
Mark