review (S) for 6911204: generated adapters with large signatures can fill up the code cache

[Tom Rodriguez]

Thanks.

tom

On Jan 20, 2010, at 12:55 PM, Vladimir Kozlov wrote:

> OK. Keep it as it is.
> 
> Vladimir
> 
> Tom Rodriguez wrote:
>> I just felt like that was too ugly for the limited benefit it would bring.  If you really want it I will put it in.  I just don't believe that making equals faster is really going to bring a measurable improvement.
>> tom
>> On Jan 20, 2010, at 12:27 PM, Vladimir Kozlov wrote:
>>> Tom,
>>> 
>>> I suggested before to use 'xor' and 'or' to avoid branches instead of :
>>> 
>>> +       return _value._compact_int[0] == other->_value._compact_int[0] &&
>>> +         _value._compact_int[1] == other->_value._compact_int[1] &&
>>> +         _value._compact_int[2] == other->_value._compact_int[2];
>>> 
>>> like this:
>>> 
>>> +       return ((other->_value._compact_int[0] ^ value._compact_int[0]) |
>>> +               (other->_value._compact_int[1] ^ value._compact_int[1]) |
>>> +               (other->_value._compact_int[2] ^ value._compact_int[2])) == 0;
>>> +     }
>>> 
>>> It should be the same result. Right?
>>> 
>>> Otherwise looks good.
>>> 
>>> Vladimir
>>> 
>>> Tom Rodriguez wrote:
>>>> So there were a couple bugs with the hash computation from when I added the compact form that I've fixed.  I also did some experimentation with hash functions and table sizes to find reasonable values and added some statistics about the lookup.  I also changed the lookup process to eliminate the C heap allocation in the normal lookup path.  This means that the finger print is computed twice in the case where a new adapter is being created but doing it that was more straightforward coding-wise.  Basically 99% of the time we find what we are looking for in the table already using the compact signature without performing any C heap allocation.  I've updated the webrev with the changes.
>>>> tom
>>>> On Jan 15, 2010, at 2:14 PM, John Rose wrote:
>>>>> Either way is fine with me.  I like Vladimir's suggestion.  -- John
>>>>> 
>>>>> On Jan 15, 2010, at 1:45 PM, Tom Rodriguez wrote:
>>>>> 
>>>>>> It's hot but not necessarily the most expensive.  We have to parse the UTF8 signature to construct an array of BasicTypes and then pass that to the calling_convention to build a VMRegPair[].  That's an interesting trick to speed up the equals test itself though.  I discovered a problem with the hash computation for the compacted form that's leading me to reevaluate the hash function itself and collect some statistics.  I'm going to fix that and maybe incorporate your faster equals.