RFR: 8329331: Intrinsify Unsafe::setMemory [v12]

[Sandhya Viswanathan]

On Thu, 11 Apr 2024 21:47:01 GMT, Scott Gibbons <sgibbons at openjdk.org> wrote:

>> This code makes an intrinsic stub for `Unsafe::setMemory` for x86_64.  See [this PR](https://github.com/openjdk/jdk/pull/16760) for discussion around this change.
>> 
>> Overall, making this an intrinsic improves overall performance of `Unsafe::setMemory` by up to 4x for all buffer sizes.
>> 
>> Tested with tier-1 (and full CI).  I've added a table of the before and after numbers for the JMH I ran (`MemorySegmentZeroUnsafe`).
>> 
>> [setMemoryBM.txt](https://github.com/openjdk/jdk/files/14808974/setMemoryBM.txt)
>
> Scott Gibbons has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Addressing more review comments

src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 2751:

> 2749:         UnsafeSetMemoryMark usmm(this, true, true);
> 2750: 
> 2751:         __ generate_fill(T_BYTE, false, c_rarg0, c_rarg1, r11, rax, xmm0);

We will be duplicating the code gen for generate_fill here? Could we not do a tail call to _jbyte_fill here and add UnsafeSetMemoryMark inside _jbyte_fill?

src/hotspot/share/opto/library_call.cpp line 4952:

> 4950: }
> 4951: 
> 4952: bool LibraryCallKit::inline_unsafe_setMemory() {

It will be good to add the signature of Unsafe.setMemory0 as a comment above line 4952.

src/hotspot/share/opto/runtime.cpp line 783:

> 781:   fields[argp++] = TypeLong::LONG;      // size
> 782:   fields[argp++] = Type::HALF;          // size
> 783:   fields[argp++] = TypeInt::INT;        // bytevalue

Should this be TypeInt::BYTE?

src/hotspot/share/runtime/sharedRuntime.cpp line 181:

> 179: 
> 180: uint SharedRuntime::_unsafe_set_memory_ctr=0;
> 181: 

Extra blank line before line 180 could be removed.

src/hotspot/share/runtime/sharedRuntime.cpp line 1994:

> 1992:   if (_rethrow_ctr) tty->print_cr("%5u rethrow handler", _rethrow_ctr);
> 1993: 
> 1994:   if (_unsafe_set_memory_ctr) tty->print_cr("%5u unsafe set memorys", _unsafe_set_memory_ctr);

Extra blank line before line 1994 could be removed.

src/hotspot/share/runtime/sharedRuntime.hpp line 546:

> 544: 
> 545:   static uint _unsafe_set_memory_ctr;      // Slow-path includes alignment checks
> 546: 

Extra blank line before line 545 could be removed.

test/jdk/sun/misc/CopyMemory.java line 214:

> 212:         random.setSeed(seed);
> 213:         System.out.println("Seed set to "+ seed);
> 214: 

Looks like these lines were added for debugging, could be removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561853120
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561831702
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561820596
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561822279
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561822556
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561823861
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1561828829