On 04/02/2016 05:18 PM, Patrick Palka wrote:
Here's a version that uses a separate deletable table to cache the
function copies. For simplicity I used a hash_map instead of a
hash_table. Does this look OK to commit after bootstrap + regtest?
Thanks. Minor nits:
> +struct fundef_copies_table_t
> +{
> + hash_map<tree, fun_copy *> *map;
> +};
Why wrap the pointer in a struct?
+ maybe_initialize_fundef_copies_table ();
+ fun_copy *copy = get_fun_copy (fun);
Let's move the initialization call inside get_fun_copy.
On a related note, I noticed that the constexpr_call_table is not marked
deletable. Marking it deletable speeds up the test case in the PR by
about 10% and saves about 10MB. Do you think doing so is a good idea?
Please.
On another related note, I noticed that marking something as both
GTY((deletable, cache)) doesn't work as intended, because the
gt_cleare_cache functions run _after_ all deletable roots get
zeroed out. So during GC the gt_cleare_cache function of a root
marked "deletable, cache" would always be a no-op. Concretely I think
this means that our cv_cache and fold_cache leak memory during GC
because their underlying hash_map (allocated by operator new) is zeroed
before gc_cleare_cache could clear it.
Hmm, I thought I remembered hitting the breakpoint in gt_cleare_cache
and it being non-null. But I guess we can get rid of the cache_map
class and use the approach you have here, of a deletable gc-allocated
hash_map pointer; I'd still use ->empty() for dumping the cache outside
of GC, though.
Jason
