RFR: Implement value type buffering for the interpreter
frederic.parain at oracle.com
Mon Jun 26 17:11:39 UTC 2017
Here's a new webrev:
- fix parameter default value in global.hpp
- add comment about VTBufferChunk fields in vtBuffer.hpp
- add two new counters to track failed chunk allocations
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?
>> On 20/06/17 22:50, Frederic Parain wrote:
>>> Here's a webrev implementing value type buffering for the interpreter.
>>> 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.
More information about the valhalla-dev