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

Vladimir Kozlov kvn at openjdk.org
Fri Apr 19 21:11:00 UTC 2024


On Fri, 19 Apr 2024 20:13:03 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:
> 
>   Review comments

This looks good. I only have question about long vs short jumps in stub's code.

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

> 2548: 
> 2549:   // If zero, then we're done
> 2550:   __ jccb(Assembler::zero, L_exit);

Code in `generate_unsafe_setmemory()` uses long jumps to `L_exit` but here you use short. Why?

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

> 2636:                                L_exit, _masm);
> 2637:     }
> 2638:     __ jmp(L_exit);

Here is long jump to `L_exit` after `do_setmemory_atomic_loop()` call. Should this be also short jump?

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

> 783:   fields[argp++] = TypePtr::NOTNULL;        // dest
> 784:   fields[argp++] = TypeX_X;                 // size
> 785:   LP64_ONLY(fields[argp++] = Type::HALF);  // size

Nit: align `/`

src/hotspot/share/utilities/copy.hpp line 2:

> 1: /*
> 2:  * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.

You forgot to undo year change in this file.

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

PR Review: https://git.openjdk.org/jdk/pull/18555#pullrequestreview-2012400269
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572947954
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572948693
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572955327
PR Review Comment: https://git.openjdk.org/jdk/pull/18555#discussion_r1572960023


More information about the core-libs-dev mailing list