RFR 8246095: Tweaks to memory access API

Wed Jun 3 14:51:07 UTC 2020

LGTM.

The illustrative example in asUnsigned is nice ( I should have thought of similar, D’oh! )

-Chris.

> On 3 Jun 2020, at 15:25, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> Please review this delta patch; I realized that the previously submitted patch had few issues:
> 
> * copyright header on TestAdaptVarHandle benchmark was wrong
> * stream test for segment spliterator was not updated to include changes to copyFrom API
> * I've also incorporated a small javadoc tweak to MemoryHandles::asUnsigned that was suggested in the CSR process
> 
> Here's the patch (if a webrev if preferred, I can also generate that):
> 
> diff -r 6b40a32ede25 src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java
> --- a/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java Wed Jun 03 13:57:18 2020 +0100
> +++ b/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java Wed Jun 03 15:22:03 2020 +0100
> @@ -357,9 +357,14 @@
>       * primitive data types as if they were a wider signed primitive type. For
>       * example, it is often convenient to model an <i>unsigned short</i> as a
>       * Java {@code int} to avoid dealing with negative values, which would be
> -     * the case if modeled as a Java {@code short}. The returned var handle
> -     * converts to and from wider primitive types, to a more narrow possibly
> -     * unsigned primitive type.
> +     * the case if modeled as a Java {@code short}. This is illustrated in the following example:
> +     * <blockquote><pre>{@code
> +    MemorySegment segment = MemorySegment.allocateNative(2);
> +    VarHandle SHORT_VH = MemoryLayouts.JAVA_SHORT.varHandle(short.class);
> +    VarHandle INT_VH = MemoryHandles.asUnsigned(SHORT_VH, int.class);
> +    SHORT_VH.set(segment.baseAddress(), (short)-1);
> +    INT_VH.get(segment.baseAddress()); // returns 65535
> +     * }</pre></blockquote>
>       * <p>
>       * When calling e.g. {@link VarHandle#set(Object...)} on the resulting var
>       * handle, the incoming value (of type {@code adaptedType}) is converted by a
> diff -r 6b40a32ede25 test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java
> --- a/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java Wed Jun 03 13:57:18 2020 +0100
> +++ b/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java Wed Jun 03 15:22:03 2020 +0100
> @@ -23,7 +23,6 @@
> 
>  package org.openjdk.tests.java.util.stream;
> 
> -import jdk.incubator.foreign.MemoryAddress;
>  import jdk.incubator.foreign.MemoryLayout;
>  import jdk.incubator.foreign.MemoryLayouts;
>  import jdk.incubator.foreign.MemorySegment;
> @@ -116,9 +115,9 @@
> 
>      static Consumer<MemorySegment> segmentCopier(Consumer<MemorySegment> input) {
>          return segment -> {
> -            MemorySegment copy = MemorySegment.ofArray(new byte[(int)segment.byteSize()]);
> -            MemoryAddress.copy(segment.baseAddress(), copy.baseAddress(), segment.byteSize());
> -            input.accept(copy);
> +            MemorySegment dest = MemorySegment.ofArray(new byte[(int)segment.byteSize()]);
> +            dest.copyFrom(segment);
> +            input.accept(dest);
>          };
>      }
> 
> diff -r 6b40a32ede25 test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java
> --- a/test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java Wed Jun 03 13:57:18 2020 +0100
> +++ b/test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java Wed Jun 03 15:22:03 2020 +0100
> @@ -1,25 +1,27 @@
>  /*
> - * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
> - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> + *  Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> + *  DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
> - * This code is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 only, as
> - * published by the Free Software Foundation.
> + *  This code is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License version 2 only, as
> + *  published by the Free Software Foundation.
>   *
> - * This code is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> - * version 2 for more details (a copy is included in the LICENSE file that
> - * accompanied this code).
> + *  This code is distributed in the hope that it will be useful, but WITHOUT
> + *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> + *  version 2 for more details (a copy is included in the LICENSE file that
> + *  accompanied this code).
>   *
> - * You should have received a copy of the GNU General Public License version
> 
> Maurizio
> 
> On 28/05/2020 22:25, Maurizio Cimadamore wrote:
>> Hi,
>> this followup change includes a number of tweaks that have been added to the memory access API while we were in the process of integrating it. Most of them have been contributed by Chris (thanks!), and are all listed in great details in the CSR below:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8246096
>> 
>> Implementation-wise it should be all relatively straightforward - apart for the bit of surgery on lambda forms which was required to add a new kind of lambda forms to 'collect' return values. For now this method handle adaptation is package private, and can only be used by MemoryHandles::filterValues - but, if people find it useful, we might consider adding it to the MethodHandle standard combinator toolbox.
>> 
>> Cheers
>> Maurizio
>> 
>> 
>> Webrev:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/webrev/
>> 
>> Javadoc:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/javadoc/jdk/incubator/foreign/package-summary.html 
>> 
>> Specdiff:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/specdiff/overview-summary.html 
>> 