On Wed, 25 Nov 2020 12:41:40 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. Looks like a good cleanup. I agree that mixing access and instantiation is not good from a benchmark perspective - although, given that direct buffers have a rather convoluted allocation process, I guess it would be useful, in the long term, to track performances of that too (this can be done by coupling direct buffer allocation with calls to Unsafe::invokeCleaner, so that the benchmark is not affected by the GC activity). Also, given a carrier e.g. `float` there are at least two ways to get a float from a byte array: * ByteBuffer::getFloat (both direct/heap) * FloatBuffer::get() (both direct/heap) The benchmark here focuses on the first bullet, but I think at some point we should also cover the remaining axis, so that, for each carrier, we really do at least 4 tests. Of course if you take into account swappiness for non-byte carriers, you get an extra variant for direct buffers as well, as you wanna test native endian vs. non-native endian. So many possibilities :-) ------------- Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1430
