[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR (XS) 8152380 - Shared symbol/string tables should never use alternate hashcode
From:       Ioi Lam <ioi.lam () oracle ! com>
Date:       2016-03-25 20:33:46
Message-ID: 56F5A0AA.5070706 () oracle ! com
[Download RAW message or body]

Thanks Coleen & Jiangli for the review. I will add the comment when I do 
the push.

- Ioi

On 3/25/16 10:54 AM, Jiangli Zhou wrote:
> +1
> 
> Thanks,
> Jiangli
> 
> > On Mar 25, 2016, at 6:47 AM, Coleen Phillimore <coleen.phillimore@oracle.com> \
> > wrote: 
> > 
> > Ioi,
> > This looks good.  Thank you for fixing this.
> > 
> > http://cr.openjdk.java.net/~iklam/jdk9/8152380_shared_table_rehashing.v02/src/share/vm/classfile/symbolTable.cpp.udiff.html
> >  
> > Can you add a 1 line comment at line 209, something like
> > // hash_code parameter may use alternate hashing algorithm but the shared table \
> > always uses the same original hash code (ie. why you have this if statement).
> > 
> > thanks,
> > Coleen
> > 
> > On 3/25/16 12:01 AM, Ioi Lam wrote:
> > > Please review a very small fix:
> > > 
> > > http://cr.openjdk.java.net/~iklam/jdk9/8152380_shared_table_rehashing.v02/
> > > https://bugs.openjdk.java.net/browse/JDK-8152380
> > > 
> > > Summary of fix:
> > > 
> > > I added a new function, SymbolTable::hash_shared_symbol(), that does not
> > > use the alternate hashing algorithm.
> > > 
> > > To validate the fix, I added new test cases that force alternate hashing
> > > while CDS is enabled (thus we have a shared symbol/string tables).
> > > Unfortunately the tests are in closed repos.
> > > 
> > > Testing showed that the problem existed only for the shared symbol table.
> > > The shared string table already did not use the alternate hashing algorithm.
> > > 
> > > Tests:
> > > 
> > > JPRT
> > > RBT with hotspot tests
> > > 
> > > Thanks
> > > - Ioi


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic