RFR: Implement value type buffering for the interpreter

[Frederic Parain]

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
> 
>