RFR: 8130115: REDO - Reduce Symbol::_identity_hash to 2 bytes

[Yumin Qi]

Ioi,

   I am trying to add a test case in SA for the testing as you 
mentioned. The easy part is adding a simple SA Tool (SymbolsInfo.java) 
to get the Symbol information  but encountered a problem as:

   In the testing java process (1), create (spawn) another java 
process(2), which will run SA (SymbolsInfo) and attach back to the 
process (1).  It failed due to time out waiting for response from 
target(1). I am investigating and trying to find a solution.  It may 
have issue for such case.

   Webrev (Note: in the webrev, WhitBox.java, white_box.cpp, 
SymbolsInfo.java and IdentityHashForSymbols.java added to previous 
version webrev01)

   http://cr.openjdk.java.net/~minqi/8130115/webrev02/

   Any one has comments how to solve the problem?
   Following are the two processes, you can see process 2) has parent as 
process 1): ($WS, $TEST are for real locations on my host machine)

1) 25939 25807 19 09:32 pts/1    00:00:00 $MYJDK/bin/java 
-Dtest.src=$WS/hotspot/test/serviceability/sa 
-Dtest.src.path=$WS/hotspot/test/serviceability/sa:$WS/hotspot/test/testlibrary:$WS/test/lib 
-Dtest.classes=$TEST/JTwork/classes/serviceability/sa 
-Dtest.class.path=$TEST/JTwork/classes/serviceability/sa:$TEST/JTwork/classes/testlibrary:$TEST/test/lib 
-Dtest.vm.opts= -Dtest.tool.vm.opts= -Dtest.compiler.opts= 
-Dtest.java.opts= -Dtest.jdk=$MYJDK -Dcompile.jdk=$MYJDK 
-Dtest.timeout.factor=1.0 -Xbootclasspath/a:. 
-XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI 
com.sun.javatest.regtest.agent.MainWrapper 
$TEST/JTwork/classes/serviceability/sa/IdentityHashForSymbols.jta

2) 25976 25939 63 09:32 pts/1    00:00:03 $MYJDK/bin/java -cp 
$TEST/jtreg/lib/javatest.jar:$TEST/jtreg/lib/jtreg.jar:$TEST/JTwork/classes/serviceability/sa:$WS/hotspot/test/serviceability/sa:$TEST/JTwork/classes/testlibrary:$TEST/test/lib 
sun.jvm.hotspot.tools.SymbolsInfo 25939


Thanks
Yumin

On 7/27/2015 9:29 PM, Ioi Lam wrote:
> Hi Yumin,
>
> The C code changes look good to me.
>
> I am a little concerned about the Java code's calculation of 
> identityHash:
>
> Java version:
>   86   public int identityHash() {
>   87     long addr_value = getAddress().asLongValue();
>   88     int  addr_bits = (int)(addr_value >> 
> (VM.getVM().getLogMinObjAlignmentInBytes() + 3));
>   89     int  length = (int)getLength();
>   90     int  byte0 = getByteAt(0);
>   91     int  byte1 = getByteAt(1);
>   92     int  id_hash = (int)(0xffff & idHash.getValue(this.addr));
>   93     return id_hash |
>   94            ((addr_bits ^ (length << 8) ^ ((byte0 << 8) | byte1)) 
> << 16);
>   95   }
>
> C version:
>  148   unsigned identity_hash() {
>  149     unsigned addr_bits = (unsigned)((uintptr_t)this >> 
> (LogMinObjAlignmentInBytes + 3));
>  150     return (unsigned)_identity_hash |
>  151            ((addr_bits ^ (_length << 8) ^ (( _body[0] << 8) | 
> _body[1])) << 16);
>  152   }
>
> The main problem is to correctly emulate the C unsigned operations in 
> the Java code. I've eyeballed the code and it seems correct, but I am 
> wondering if you have actually tested and verified that the Java 
> version indeed returns the same value as the C code? A unit test case 
> would be good:
>
>  * Add a new test in hotspot/agent/test
>  * Get a few Symbols (e.g., call
>    sun.jvm.hotspot.runtime.VM.getSymbolTable and iterate over the first
>    1000 Symbols)
>  * For each Symbol, call its Symbol.identityHash() method
>  * Add a new whitebox API to return the C version of the
>    identity_hash() value
>  * Check if the C value is the same as the Java value
>
> Please run the test on all platforms (both 32-bit and 64-bit, and all 
> OSes).
>
> Thanks
> - Ioi
>
>
> On 7/15/15 10:37 AM, Yumin Qi wrote:
>> Hi,
>>
>>   This is redo for bug 8087143, in that push, it caused failure on 
>> Serviceability Agent failed to get type for "_identity_hash": 
>> mistakenly used JShortField for it, but in fact it still is 
>> CIntegerField. In this change, besides of the original change in 
>> hotspot/src, I add code to calculate identity_hash in hotspot/agent 
>> based on the changed in hotspot.
>>
>> Old webrev for 8087143:
>> bug: https://bugs.openjdk.java.net/browse/JDK-8087143
>> webrev: http://cr.openjdk.java.net/~minqi/8087143/webrev03/
>>
>> Summary: _identity_hash is an integer in Symbol (SymbolBase), it is 
>> used to compute hash bucket index by modulus division of table size. 
>> Currently in hotspot, no table size is more than 65535 so we can use 
>> short instead.  For case with table size over 65535 we can use the 
>> first two bytes of symbol data to be as the upper 16 bits for the 
>> calculation but rare cases.
>>
>> New webrev for 8130115:
>> bug: https://bugs.openjdk.java.net/browse/JDK-8130115
>> webrev: http://cr.openjdk.java.net/~minqi/8130115/webrev01/
>>
>>
>> Tests: JPRT, SA manual tests, -atk quick, jtreg hotspot/runtime
>> Also internal large application used for hashtable data analysis --- 
>> the No. of loaded classes is big(over 19K), and tested with different 
>> bucket sizes including over 65535 to see the new algorithm for 
>> identity_hash calculation, result shows the consistency before and 
>> after the fix.
>>
>> Thanks
>> Yumin
>