Skip to content

Commit

Permalink
8333884: MemorySegment::reinterpret removes read-only property
Browse files Browse the repository at this point in the history 
Reviewed-by: jvernee, mcimadamore
  • Loading branch information
@minborg
minborg committed Jul 6, 2024
1 parent b83766e commit 6f7f0f1
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 8 deletions.
21 changes: 21 additions & 0 deletions src/java.base/share/classes/java/lang/foreign/MemorySegment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* asSlice(offset, newSize, 1);
* }
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand All @@ -652,6 +655,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* alignment constraint. The returned segment's address is the address of this
* segment plus the given offset; its size is specified by the given argument.
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand Down Expand Up @@ -679,6 +685,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* asSlice(offset, layout.byteSize(), layout.byteAlignment());
* }
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand All @@ -705,6 +714,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* asSlice(offset, byteSize() - offset);
* }
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand All @@ -721,6 +733,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* Returns a new memory segment that has the same address and scope as this segment,
* but with the provided size.
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand Down Expand Up @@ -759,6 +774,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* scope, and is accessible from any thread. The size of the segment accepted by the
* cleanup action is {@link #byteSize()}.
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand Down Expand Up @@ -807,6 +825,9 @@ public sealed interface MemorySegment permits AbstractMemorySegmentImpl {
* scope, and is accessible from any thread. The size of the segment accepted by the
* cleanup action is {@code newSize}.
* <p>
* If this segment is {@linkplain MemorySegment#isReadOnly() read-only},
* the returned segment is also {@linkplain MemorySegment#isReadOnly() read-only}.
* <p>
* The returned memory segment shares a region of backing memory with this segment.
* Hence, no memory will be allocated or freed by this method.
*
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public MemorySegment reinterpretInternal(Class<?> callerClass, long newSize, Sco
() -> cleanup.accept(SegmentFactories.makeNativeSegmentUnchecked(address(), newSize)) :
null;
return SegmentFactories.makeNativeSegmentUnchecked(address(), newSize,
(MemorySessionImpl)scope, action);
(MemorySessionImpl)scope, readOnly, action);
}

private AbstractMemorySegmentImpl asSliceNoCheck(long offset, long newSize) {
Expand Down
10 changes: 7 additions & 3 deletions src/java.base/share/classes/jdk/internal/foreign/SegmentFactories.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2024, 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
Expand Down Expand Up @@ -57,14 +57,18 @@ public class SegmentFactories {
// associated with MemorySegment::ofAddress.

@ForceInline
public static MemorySegment makeNativeSegmentUnchecked(long min, long byteSize, MemorySessionImpl sessionImpl, Runnable action) {
public static MemorySegment makeNativeSegmentUnchecked(long min,
long byteSize,
MemorySessionImpl sessionImpl,
boolean readOnly,
Runnable action) {
ensureInitialized();
if (action == null) {
sessionImpl.checkValidState();
} else {
sessionImpl.addCloseAction(action);
}
return new NativeMemorySegmentImpl(min, byteSize, false, sessionImpl);
return new NativeMemorySegmentImpl(min, byteSize, readOnly, sessionImpl);
}

@ForceInline
Expand Down
12 changes: 8 additions & 4 deletions test/jdk/java/foreign/TestSegments.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, 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
Expand Down Expand Up @@ -43,7 +43,6 @@
import java.util.function.Supplier;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.JAVA_LONG;
import static org.testng.Assert.*;

public class TestSegments {
Expand Down Expand Up @@ -383,9 +382,14 @@ void testReinterpret() {
assertEquals(MemorySegment.ofAddress(42).reinterpret(100, Arena.ofAuto(), null).byteSize(), 100);
// check scope and cleanup
assertEquals(MemorySegment.ofAddress(42).reinterpret(100, arena, s -> counter.incrementAndGet()).scope(), arena.scope());
assertEquals(MemorySegment.ofAddress(42).reinterpret(arena, s -> counter.incrementAndGet()).scope(), arena.scope());
assertEquals(MemorySegment.ofAddress(42).reinterpret(arena, _ -> counter.incrementAndGet()).scope(), arena.scope());
// check read-only state
assertFalse(MemorySegment.ofAddress(42).reinterpret(100).isReadOnly());
assertTrue(MemorySegment.ofAddress(42).asReadOnly().reinterpret(100).isReadOnly());
assertTrue(MemorySegment.ofAddress(42).asReadOnly().reinterpret(100, Arena.ofAuto(), null).isReadOnly());
assertTrue(MemorySegment.ofAddress(42).asReadOnly().reinterpret(arena, _ -> counter.incrementAndGet()).isReadOnly());
}
assertEquals(counter.get(), 2);
assertEquals(counter.get(), 3);
}

@Test
Expand Down

7 comments on commit 6f7f0f1

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@JornVernee
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk23

@openjdk
Copy link

@openjdk openjdk bot commented on 6f7f0f1 Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JornVernee The target repository jdk23 is not a valid target for backports.
List of valid target repositories: openjdk/jdk, openjdk/jdk11u, openjdk/jdk11u-dev, openjdk/jdk17u, openjdk/jdk17u-dev, openjdk/jdk21u, openjdk/jdk21u-dev, openjdk/jdk22u, openjdk/jdk23u, openjdk/jdk7u, openjdk/jdk8u, openjdk/jdk8u-dev, openjdk/jfx, openjdk/jfx17u, openjdk/jfx21u, openjdk/jfx22u, openjdk/lilliput-jdk17u, openjdk/lilliput-jdk21u, openjdk/shenandoah-jdk21u, openjdk/shenandoah-jdk8u.
Supplying the organization/group prefix is optional.

There is a branch jdk23 in the current repository openjdk/jdk.
To target a backport to this branch in the current repository use:
/backport :jdk23

@JornVernee
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport :jdk23

@openjdk
Copy link

@openjdk openjdk bot commented on 6f7f0f1 Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JornVernee Could not automatically backport 6f7f0f1d to openjdk/jdk due to conflicts in the following files:

  • src/java.base/share/classes/java/lang/foreign/MemorySegment.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk.git jdk23:jdk23

# Check out the target branch and create your own branch to backport
$ git checkout jdk23
$ git checkout -b backport-JornVernee-6f7f0f1d-jdk23

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 6f7f0f1de05fdc0f6a88ccd90b806e8a5c5074ef

# Backport the commit
$ git cherry-pick --no-commit 6f7f0f1de05fdc0f6a88ccd90b806e8a5c5074ef
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 6f7f0f1de05fdc0f6a88ccd90b806e8a5c5074ef'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk with the title Backport 6f7f0f1de05fdc0f6a88ccd90b806e8a5c5074ef.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 6f7f0f1d from the openjdk/jdk repository.

The commit being backported was authored by Per Minborg on 6 Jul 2024 and was reviewed by Jorn Vernee and Maurizio Cimadamore.

Thanks!

@JornVernee
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport :jdk23

@openjdk
Copy link

@openjdk openjdk bot commented on 6f7f0f1 Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JornVernee the backport was successfully created on the branch backport-JornVernee-6f7f0f1d-jdk23 in my personal fork of openjdk/jdk. To create a pull request with this backport targeting openjdk/jdk:jdk23, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 6f7f0f1d from the openjdk/jdk repository.

The commit being backported was authored by Per Minborg on 6 Jul 2024 and was reviewed by Jorn Vernee and Maurizio Cimadamore.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk:

$ git fetch https://github.com/openjdk-bots/jdk.git backport-JornVernee-6f7f0f1d-jdk23:backport-JornVernee-6f7f0f1d-jdk23
$ git checkout backport-JornVernee-6f7f0f1d-jdk23
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk.git backport-JornVernee-6f7f0f1d-jdk23

Please sign in to comment.