RFR: Implement value type buffering for the interpreter

[Frederic Parain]

Thank you!

Fred


> On Jun 29, 2017, at 09:02, David Simms <david.simms at oracle.com> wrote:
> 
> 
> Brilliant, great work on the comments !
> 
> Push it !
> 
> /Mr. Simms
> 
> 
> On 26/06/17 19:11, Frederic Parain wrote:
>> 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
>>>> 
>>>> 
>