Re: [PATCH] c++: cache conversion function lookup

[Jason Merrill <jason@redhat.com>]

On 9/7/23 16:12, Patrick Palka wrote:
> On Thu, 7 Sep 2023, Jason Merrill wrote:
> 
>> On 9/6/23 18:07, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk?  This cache apparently has a 98% hit rate for TYPE_HAS_CONVERSION
>>> types on some test files.
>>
>> Does it make a measurable difference in compile time?
> 
> Admittedly this optimization was probably more justified with the older
> version of the PR99599 patch that added another lookup_conversions call.
> Compile time for this standalone patch in the noise according to my
> experiments, but I did measure a ~1MB/0.2% decrease in memory usage for
> range-v3's zip.cpp.  This is because lookup_conversions_r allocates a
> new list each time it's called (on a TYPE_HAS_CONVERSION type) even in
> the simple case of no inheritance etc.  Maybe lookup_conversions_r's
> (relatively complicated) implementation could be improved in the
> simple/common case...
> 
>>> +/* A cache of the result of lookup_conversions.  */
>>> +
>>> +static GTY((cache)) type_tree_cache_map *lookup_conversions_cache;
>>
>> This should probably be (deletable) rather than (cache)?
> 
> Ack.  Is that because of PCH concerns or because the cache is
> purely an optimization and so has no semantic effect?

The latter.  Really, (cache) is a terrible name, it should only be used 
when it's impractical to recompute the value.

Jason