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

[Coleen Phillimore]

This looks okay to me also.  The renaming of "probe" to 
"lookup_and_ignore_hash" seems odd to me since nowhere in the caller do 
you suspect there's a hash code of any interest.  I would have picked 
simply "lookup" since the old "lookup" turned into "lookup_and_add".  If 
this makes it clearer to you while working on this code, then it is fine.

Coleen

On 4/30/14, 3:56 PM, Gerard Ziemski wrote:
> 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.
>>>
>>>
>>>
>>>
>>
>