RFR (round 2) JDK-8039525: HashTable lookup API needs refactoring

[Gerard Ziemski]

Thanks for the review.

You have to be familiar with the base API to know that it takes hash. 
The idea behind these changes is to clarify, and they are not required 
as you noted. This also is a first stab at this, we can come and revisit 
later, but let's just make a bit of progress now.


cheers

On 4/30/2014 1:58 PM, Christian Thalinger wrote:
> One more comment: why do we need the _ignore_hash versions?  The 
> method signature already implies that the user is ignoring it:
>
> *!    static Symbol*_lookup_and_hash_(const char* name, int len, unsigned int& hash);*
> *!    static Symbol*_lookup_and_ignore_hash_(const char* name, int len) {*
>
> That also questions the _and_hash versions.  Anyway, maybe I’m not 
> understanding the purpose of this change.  Whatever you do the change 
> looks good.
>
> On Apr 30, 2014, at 8:00 AM, Gerard Ziemski <gerard.ziemski at oracle.com 
> <mailto:gerard.ziemski at oracle.com>> wrote:
>
>> hi all,
>>
>> Please review this 2nd webrev for a simple enhancement, which renames 
>> symbolTable::lookup
>> API using names that better reflect the method purpose, ie: lookup, 
>> lookup_and_add,
>> lookup_and_hash, lookup_and_add_unicode, lookup_and_hash_unicode and 
>> lookup_and_ignore_hash.
>>
>>
>> The changes since webrev 1:
>>
>> Re: Christian Thalinger
>>
>> Dropped the “_only” suffix completely - I decided it was no longer 
>> needed after being left
>> with 1 API using it (ie. lookup_only —> lookup, after dropping it 
>> first from lookup_only_and_add)
>>
>> Re: John Rose
>>
>> Renamed “probe” into “lookup_and_ignore_hash”. Removed “blank” lines 
>> and checked with jcheck
>>
>> Re: Per Liden
>>
>> Decided to leave “lookup_and_add” alone, since “lookup_or_add” 
>> implies one or the other and seems
>> even less right. We could have renamed “lookup_and_add” as 
>> “lookup_and_add_if_needed”,
>> but there is no “lookup_and_add_always”, so I simply left it alone - 
>> hope that’s OK.
>>
>>
>> Webrev: http://cr.openjdk.java.net/~gziemski/8039525_rev1 
>> <http://cr.openjdk.java.net/%7Egziemski/8039525_rev1>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8039525
>>
>> Tested with hotspot jtreg, vm.quicklooks and JPRT. Locally builds on 
>> Mac OS X and Linux.
>>
>> Thank you.
>>
>>
>>
>>
>