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

Christian Thalinger christian.thalinger at oracle.com
Wed Apr 30 18:58:27 UTC 2014


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> 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
> 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.
> 
> 
> 
> 



More information about the hotspot-runtime-dev mailing list