Trevor Saunders <tbsau...@tbsaunde.org> writes:
> On Tue, Jun 16, 2015 at 09:45:56AM +0100, Richard Sandiford wrote:
>> I couldn't tell whether the unusual name of the function
>> ("gt_cleare_cache") is deliberate or not, but I left it be.
>
> I'm not sure what's particularly unusual about it?
"cleare" rather than "clear".
>> + static int
>> + keep_cache_entry (tree_int_map *&m)
>
> I think we could now change the interface to take const T *? I imagine
> inlining may get rid of the extra indirection anyway, but it feels
> cleaner anyway.
Yeah, good point.
>> + - An optional static function named 'keep_cache_entry'. This
>> + function is provided only for garbage-collected elements that
>> + are not marked by the normal gc mark pass. It describes what
>> + what should happen to the element at the end of the gc mark phase.
>> + The return value should be:
>> + - 0 if the element should be deleted
>> + - 1 if the element should be kept and needs to be marked
>> + - -1 if the element should be kept and is already marked.
>> + Returning -1 rather than 1 is purely an optimization.
>
> In theory using an enum seems better, but I'm not sure if the extra
> verbosity makes it better in practice.
Yeah, I wondered about an enum but it seemed like overkill.
>> + static int
>> + keep_cache_entry (T &e)
>> {
>> - if (e != HTAB_EMPTY_ENTRY && e != HTAB_DELETED_ENTRY &&
>> !ggc_marked_p (e))
>> - e = static_cast<T> (HTAB_DELETED_ENTRY);
>> + return ggc_marked_p (e) ? -1 : 0;
>
> hmm, this is the only place where -1 is used right? I believe this
> case only works if the hash table is storing pointers to things in gc
> memory, and only keeps things that get marked by other paths. I
> believe that means -1 is only a very small optimization because in the
> case we return -1 all we save is the check that that pointer itself is
> marked.
Right. But it's an optimisation that we had before and I think we
should keep it. The -1 case is in the generic traits rather than
"user" code.
> So i'm tempted to change this interface to just return a bool.
> Of course it would be nice if the compiler could inline enough to
> actually optimize out the redundant check if the pointer is makred, but
> that might take some shuffling code around.
In practice all calls will return from {0, 1} or {0, -1}, so assuming
sensible inlining, no caller will use all three paths.
Thanks,
Richard
