RFR: Implement value type buffering for the interpreter

[Frederic Parain]

Here's a new webrev:

http://cr.openjdk.java.net/~fparain/vt-buffering/webrev.02/index.html

Changes:

   - fix parameter default value in global.hpp
   - add comment about VTBufferChunk fields in vtBuffer.hpp
   - add two new counters to track failed chunk allocations

Thanks,

Fred

On 06/26/2017 10:40 AM, Frederic Parain wrote:
> Mr Simms,
> 
> Thank you for the review.
> See my answers in-lined below.
> 
> On 06/26/2017 10:04 AM, David Simms wrote:
>>
>> Kudos on the loop testing, like the targeted testing . Good work on 
>> the static init fixes and testing.
>>
>> Minor comments:
>>
>>   * globals.hpp:4097 "sizeof(long long)", is at least 64bits, but not
>>     fixed size: do you mean BytesPerLong or longSize ?
> 
> Changed to BytesPerLong.
> Eventually, this parameter would become a per-platform parameter, to
> take into account the instruction capabilities of each CPU.
> 
>>   * ValueTypesBufferMaxMemory=128, global fixed limit
>>       o "doesn't feel right", but I admit to not having a better answer.
>>         Needs more user testing: TODO, we did talk off list about this,
>>         discussed combination of global + per-thread options.
> 
> I've delayed this work to push the VT buffering ASAP.
> Among the TODO list, I'd like to re-write the backend allocator to
> avoid fragmenting the memory. I'd like to implement the global/local
> limits we have discussed, but I need more usage data to tune them.
> 
>>       o Should "Global VTBuffer Pool statistics" include a failure
>>         statistic to help with performance evaluation/tuning ?
> 
> Nice suggestion.
> A global allocating failure counter could help detecting too small
> global buffer size.
> In addition, a per-thread allocation failure counter could help
> detecting unbalanced use of the global buffer memory.
> I'll implement it this week.
> 
>>   * vtBuffer.hpp:40 "_index;" - I can guess what all the other fields
>>     are at a glance, but a short comment here would be nice
> 
> Each thread builds its own linked list of VTBufferChunk. Having an
> index in each chunk indicating its position in the list enables
> optimizations when comparing the addresses of two buffered values
> (because buffer chunks are not contiguous, addresses cannot be
> compared directly).
> I'll add a comment about that.
> 
>> I was wondering if "InterpreterFrameClosure" changes didn't trigger a 
>> problem in the ValueOops whitebox testing (frame oop maps)...that test 
>> feels a little on the fragile side.
> 
> The modification of the InterpreterFrameClosure changes the way
> references to values are handled, but the oopmap themselves are
> not changed. Do you think this would cause an issue?
> 
> Thanks,
> 
> Fred
> 
>>
>> On 20/06/17 22:50, Frederic Parain wrote:
>>> Greetings,
>>>
>>> Here's a webrev implementing value type buffering for the interpreter.
>>>
>>> http://cr.openjdk.java.net/~fparain/vt-buffering/webrev.01/
>>>
>>> Buffering allows the interpreter to allocate values in a thread local
>>> buffer to avoid Java heap allocations. Memory recycling is performed
>>> on method exit and sometimes on backward branches.
>>>
>>> Format of buffered values is almost identical to the format of
>>> Java heap allocated values, so most code won't see any difference
>>> between a buffered value and a not buffered value. The only difference
>>> is in the first word of the header. Because of a change in GC closures
>>> in JDK10, the first word now stores a reference to the Java mirror
>>> of the value class in order to keep it alive. In JDK9, this operation
>>> was performed through the klass pointer in the header, but the GC
>>> team has removed this closure.
>>>
>>> Buffering can be monitored using NMT or a new diagnostic command.
>>>
>>> All tests pass and Sergey has already tested the patch with his
>>> benchmark (and reported several bugs that are now fixed, thank
>>> you Sergey).
>>>
>>> Unfortunately, the changeset also includes a number of fixes not
>>> related to buffering, like code clean up and access to uninitialized
>>> static value fields.
>>>
>>> Thanks,
>>>
>>> Fred
>>
>>