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