On Thu, 26 Nov 2020 14:38:17 GMT, Chris Hegarty <che...@openjdk.org> wrote:

>> The ByteBuffers micro benchmark seems to be a little dated. 
>> 
>> It should be a useful resource to leverage when analysing the performance 
>> impact of any potential implementation changes in the byte buffer classes. 
>> More specifically, the impact of such changes on the performance of sharp 
>> memory access operations. 
>> 
>> This issue proposes to update the benchmark in the following ways to meet 
>> the aforementioned use-case: 
>> 
>> 1. Remove allocation from the individual benchmarks - it just creates noise. 
>> 2. Consolidate per-thread shared heap and direct buffers. 
>> 3. All scenarios now use absolute memory access operations - so no state of 
>> the shared buffers is considered. 
>> 4. Provide more reasonable default fork, warmup, etc, out-of-the-box. 
>> 5. There seems to have been an existing bug in the test where the size 
>> parameter was not always considered - this is now fixed.
>
> Chris Hegarty has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   whitespace

Looks a great improvements. Two comments:

* the names always mention the "Single" word, when in fact all benchmark 
involve some kind of a loop. I'd suggest making that more  explicit, both in 
the benchmark method and in the helpers.

* I see that you added  support for views - but note that, for heap buffers 
there are two different kind of views (!!). E.g. I believe that:

ByteBuffer.allocate(size * 4).asFloatBuffer()
returns a buffer of a different class than:

FloatBuffer.allocate(size);
I think you are only testing the former?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1430

Reply via email to