On Tue, 26 Sep 2023 21:28:43 GMT, Paul Sandoz <psan...@openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix typos
>
> src/java.base/share/classes/java/lang/foreign/Linker.java line 35:
> 
>> 33: 
>> 34: import java.lang.invoke.MethodHandle;
>> 35: import java.nio.ByteOrder;
> 
> Unused?

Yes, I'll remove it.

> src/java.base/share/classes/java/lang/foreign/Linker.java line 735:
> 
>> 733:          *
>> 734:          * @apiNote This linker option can not be combined with {@link 
>> #critical}.
>> 735:          *
> 
> That seems more specification (that can be asserted on) then an informative 
> note.

True. I'll fold it into the main spec body.

> src/java.base/share/classes/jdk/internal/foreign/NativeMemorySegmentImpl.java 
> line 152:
> 
>> 150:     private static long allocateMemoryWrapper(long size) {
>> 151:         try {
>> 152:             return UNSAFE.allocateMemory(size);
> 
> Since we now zero memory only when needed we should test very carefully.

Yes. The `makeNativeSegment` is currently only called from ArenaImpl, which is 
also responsible for zeroing the memory.

I'll rename `makeNativeSegment` to `makeNativeSegmentNoZeroing` to make it 
extra clear for callers that memory will not be zeroed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1337898179
PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1337898017
PR Review Comment: https://git.openjdk.org/jdk/pull/15103#discussion_r1337899265

Reply via email to