-
Couldn't load subscription status.
- Fork 6.1k
8348556: Inlining fails earlier for MemorySegment::reinterpret #23460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6cda762
3d6a5fc
ca1dcbd
1bf5297
c095eb1
7a2f7f8
7b29824
8c5c07d
401d9d4
f715b2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. | ||
| * Copyright (c) 2014, 2025, 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 | ||
|
|
@@ -40,7 +40,6 @@ | |
| import java.net.URI; | ||
| import java.net.URL; | ||
| import java.security.CodeSource; | ||
| import java.security.ProtectionDomain; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
|
|
@@ -69,6 +68,8 @@ | |
| import jdk.internal.module.Resources; | ||
| import jdk.internal.reflect.CallerSensitive; | ||
| import jdk.internal.reflect.Reflection; | ||
| import jdk.internal.vm.annotation.DontInline; | ||
| import jdk.internal.vm.annotation.ForceInline; | ||
| import jdk.internal.vm.annotation.Stable; | ||
|
|
||
| /** | ||
|
|
@@ -176,6 +177,7 @@ public final class Module implements AnnotatedElement { | |
| * @see ClassLoader#getUnnamedModule() | ||
| * @jls 7.7.5 Unnamed Modules | ||
| */ | ||
| @ForceInline | ||
| public boolean isNamed() { | ||
| return name != null; | ||
| } | ||
|
|
@@ -186,6 +188,7 @@ public boolean isNamed() { | |
| * | ||
| * @return The module name | ||
| */ | ||
| @ForceInline | ||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
@@ -195,6 +198,7 @@ public String getName() { | |
| * | ||
| * @return The class loader for this module | ||
| */ | ||
| @ForceInline | ||
| public ClassLoader getClassLoader() { | ||
| return loader; | ||
| } | ||
|
|
@@ -205,6 +209,7 @@ public ClassLoader getClassLoader() { | |
| * | ||
| * @return The module descriptor for this module | ||
| */ | ||
| @ForceInline | ||
| public ModuleDescriptor getDescriptor() { | ||
| return descriptor; | ||
| } | ||
|
|
@@ -224,6 +229,7 @@ public ModuleDescriptor getDescriptor() { | |
| * | ||
| * @see java.lang.reflect.Proxy | ||
| */ | ||
| @ForceInline | ||
| public ModuleLayer getLayer() { | ||
| if (isNamed()) { | ||
| ModuleLayer layer = this.layer; | ||
|
|
@@ -253,6 +259,7 @@ Module implAddEnableNativeAccess() { | |
| * @return {@code true} if this module can access <em>restricted</em> methods. | ||
| * @since 22 | ||
| */ | ||
| @ForceInline | ||
| public boolean isNativeAccessEnabled() { | ||
| Module target = moduleForNativeAccess(); | ||
| return EnableNativeAccess.isNativeAccessEnabled(target); | ||
|
|
@@ -269,8 +276,9 @@ private EnableNativeAccess() {} | |
| private static final Unsafe UNSAFE = Unsafe.getUnsafe(); | ||
| private static final long FIELD_OFFSET = UNSAFE.objectFieldOffset(Module.class, "enableNativeAccess"); | ||
|
|
||
| @ForceInline | ||
| private static boolean isNativeAccessEnabled(Module target) { | ||
| return UNSAFE.getBooleanVolatile(target, FIELD_OFFSET); | ||
| return target.enableNativeAccess || UNSAFE.getBooleanVolatile(target, FIELD_OFFSET); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tries plain memory semantics first. |
||
| } | ||
|
|
||
| // Atomically sets enableNativeAccess if not already set | ||
|
|
@@ -282,53 +290,70 @@ private static boolean trySetEnableNativeAccess(Module target) { | |
|
|
||
| // Returns the Module object that holds the enableNativeAccess | ||
| // flag for this module. | ||
| @ForceInline | ||
| private Module moduleForNativeAccess() { | ||
| return isNamed() ? this : ALL_UNNAMED_MODULE; | ||
| } | ||
|
|
||
| // This is invoked from Reflection.ensureNativeAccess | ||
| @ForceInline | ||
| void ensureNativeAccess(Class<?> owner, String methodName, Class<?> currentClass, boolean jni) { | ||
| // The target module whose enableNativeAccess flag is ensured | ||
| Module target = moduleForNativeAccess(); | ||
| ModuleBootstrap.IllegalNativeAccess illegalNativeAccess = ModuleBootstrap.illegalNativeAccess(); | ||
| if (illegalNativeAccess != ModuleBootstrap.IllegalNativeAccess.ALLOW && | ||
| !EnableNativeAccess.isNativeAccessEnabled(target)) { | ||
| String mod = isNamed() ? "module " + getName() : "an unnamed module"; | ||
| if (currentClass != null) { | ||
| // try to extract location of the current class (e.g. jar or folder) | ||
| CodeSource cs = currentClass.getProtectionDomain().getCodeSource(); | ||
| if (cs != null) { | ||
| URL url = cs.getLocation(); | ||
| if (url != null) { | ||
| mod += " (" + url + ")"; | ||
| } | ||
| } | ||
| } | ||
| if (illegalNativeAccess == ModuleBootstrap.IllegalNativeAccess.DENY) { | ||
| throw new IllegalCallerException("Illegal native access from " + mod); | ||
| } else if (EnableNativeAccess.trySetEnableNativeAccess(target)) { | ||
| // warn and set flag, so that only one warning is reported per module | ||
| String cls = owner.getName(); | ||
| String mtd = cls + "::" + methodName; | ||
| String modflag = isNamed() ? getName() : "ALL-UNNAMED"; | ||
| String caller = currentClass != null ? currentClass.getName() : "code"; | ||
| if (jni) { | ||
| VM.initialErr().printf(""" | ||
| ensureNativeAccessSlowPath(owner, methodName, currentClass, jni, target, illegalNativeAccess); | ||
| } | ||
| } | ||
|
|
||
| @DontInline | ||
| void ensureNativeAccessSlowPath(Class<?> owner, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is slow anyhow so we do not need it to be inlined. |
||
| String methodName, | ||
| Class<?> currentClass, | ||
| boolean jni, | ||
| Module target, | ||
| ModuleBootstrap.IllegalNativeAccess illegalNativeAccess) { | ||
| String modDeclaredLabel = modDeclaredLabel(currentClass); | ||
| if (illegalNativeAccess == ModuleBootstrap.IllegalNativeAccess.DENY) { | ||
| throw new IllegalCallerException("Illegal native access from " + modDeclaredLabel); | ||
| } else if (EnableNativeAccess.trySetEnableNativeAccess(target)) { | ||
| // warn and set flag, so that only one warning is reported per module | ||
| String cls = owner.getName(); | ||
| String mtd = cls + "::" + methodName; | ||
| String modFlag = isNamed() ? getName() : "ALL-UNNAMED"; | ||
| String caller = currentClass != null ? currentClass.getName() : "code"; | ||
| if (jni) { | ||
| VM.initialErr().printf(""" | ||
| WARNING: A native method in %s has been bound | ||
| WARNING: %s is declared in %s | ||
| WARNING: Use --enable-native-access=%s to avoid a warning for native methods declared in this module | ||
| WARNING: Restricted methods will be blocked in a future release unless native access is enabled | ||
| %n""", cls, mtd, mod, modflag); | ||
| } else { | ||
| VM.initialErr().printf(""" | ||
| %n""", cls, mtd, modDeclaredLabel, modFlag); | ||
| } else { | ||
| VM.initialErr().printf(""" | ||
| WARNING: A restricted method in %s has been called | ||
| WARNING: %s has been called by %s in %s | ||
| WARNING: Use --enable-native-access=%s to avoid a warning for callers in this module | ||
| WARNING: Restricted methods will be blocked in a future release unless native access is enabled | ||
| %n""", cls, mtd, caller, mod, modflag); | ||
| %n""", cls, mtd, caller, modDeclaredLabel, modFlag); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private String modDeclaredLabel(Class<?> currentClass) { | ||
| String label = isNamed() ? "module " + getName() : "an unnamed module"; | ||
| if (currentClass != null) { | ||
| // try to extract location of the current class (e.g. jar or folder) | ||
| CodeSource cs = currentClass.getProtectionDomain().getCodeSource(); | ||
| if (cs != null) { | ||
| URL url = cs.getLocation(); | ||
| if (url != null) { | ||
| label += " (" + url + ")"; | ||
| } | ||
| } | ||
| } | ||
| return label; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -210,6 +210,7 @@ public void checkValidStateRaw() { | |
| * @throws IllegalStateException if this session is already closed or if this is | ||
| * a confined session and this method is called outside the owner thread. | ||
| */ | ||
| @ForceInline | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume this is added because it's called by the reinterpret implementation? |
||
| public void checkValidState() { | ||
| try { | ||
| checkValidStateRaw(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,12 +38,15 @@ | |
| */ | ||
| public sealed class NativeMemorySegmentImpl extends AbstractMemorySegmentImpl permits MappedMemorySegmentImpl { | ||
|
|
||
| private static final boolean ADDRESS_SIZE_IS_4 = | ||
| Unsafe.getUnsafe().addressSize() == 4; | ||
|
|
||
| final long min; | ||
|
|
||
| @ForceInline | ||
| NativeMemorySegmentImpl(long min, long length, boolean readOnly, MemorySessionImpl scope) { | ||
| super(length, readOnly, scope); | ||
| this.min = (Unsafe.getUnsafe().addressSize() == 4) | ||
| this.min = ADDRESS_SIZE_IS_4 | ||
| // On 32-bit systems, normalize the upper unused 32-bits to zero | ||
| ? min & 0x0000_0000_FFFF_FFFFL | ||
| // On 64-bit systems, all the bits are used | ||
|
|
@@ -77,6 +80,7 @@ ByteBuffer makeByteBuffer() { | |
| return NIO_ACCESS.newDirectByteBuffer(min, (int) this.length, null, this); | ||
| } | ||
|
|
||
| @ForceInline | ||
| @Override | ||
| public boolean isNative() { | ||
| return true; | ||
|
|
@@ -93,6 +97,7 @@ public Object unsafeGetBase() { | |
| } | ||
|
|
||
| @Override | ||
| @ForceInline | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this is needed -- this is an "accessor" method - C2 typically inline those. But, do we depend on |
||
| public long maxAlignMask() { | ||
| return 0; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3567,6 +3567,7 @@ public final long getLongUnaligned(Object o, long offset) { | |
| * @return the value fetched from the indicated object | ||
| * @since 9 | ||
| */ | ||
| @ForceInline | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These annotations are not totally out of the question as the corresponding methods (e.g. |
||
| public final long getLongUnaligned(Object o, long offset, boolean bigEndian) { | ||
| return convEndian(bigEndian, getLongUnaligned(o, offset)); | ||
| } | ||
|
|
@@ -3587,6 +3588,7 @@ public final int getIntUnaligned(Object o, long offset) { | |
| } | ||
| } | ||
| /** @see #getLongUnaligned(Object, long, boolean) */ | ||
| @ForceInline | ||
| public final int getIntUnaligned(Object o, long offset, boolean bigEndian) { | ||
| return convEndian(bigEndian, getIntUnaligned(o, offset)); | ||
| } | ||
|
|
@@ -3602,6 +3604,7 @@ public final short getShortUnaligned(Object o, long offset) { | |
| } | ||
| } | ||
| /** @see #getLongUnaligned(Object, long, boolean) */ | ||
| @ForceInline | ||
| public final short getShortUnaligned(Object o, long offset, boolean bigEndian) { | ||
| return convEndian(bigEndian, getShortUnaligned(o, offset)); | ||
| } | ||
|
|
@@ -3618,6 +3621,7 @@ public final char getCharUnaligned(Object o, long offset) { | |
| } | ||
|
|
||
| /** @see #getLongUnaligned(Object, long, boolean) */ | ||
| @ForceInline | ||
| public final char getCharUnaligned(Object o, long offset, boolean bigEndian) { | ||
| return convEndian(bigEndian, getCharUnaligned(o, offset)); | ||
| } | ||
|
|
@@ -3688,6 +3692,7 @@ public final void putLongUnaligned(Object o, long offset, long x) { | |
| * {@link NullPointerException} | ||
| * @since 9 | ||
| */ | ||
| @ForceInline | ||
| public final void putLongUnaligned(Object o, long offset, long x, boolean bigEndian) { | ||
| putLongUnaligned(o, offset, convEndian(bigEndian, x)); | ||
| } | ||
|
|
@@ -3710,6 +3715,7 @@ public final void putIntUnaligned(Object o, long offset, int x) { | |
| } | ||
| } | ||
| /** @see #putLongUnaligned(Object, long, long, boolean) */ | ||
| @ForceInline | ||
| public final void putIntUnaligned(Object o, long offset, int x, boolean bigEndian) { | ||
| putIntUnaligned(o, offset, convEndian(bigEndian, x)); | ||
| } | ||
|
|
@@ -3726,6 +3732,7 @@ public final void putShortUnaligned(Object o, long offset, short x) { | |
| } | ||
| } | ||
| /** @see #putLongUnaligned(Object, long, long, boolean) */ | ||
| @ForceInline | ||
| public final void putShortUnaligned(Object o, long offset, short x, boolean bigEndian) { | ||
| putShortUnaligned(o, offset, convEndian(bigEndian, x)); | ||
| } | ||
|
|
@@ -3736,6 +3743,7 @@ public final void putCharUnaligned(Object o, long offset, char x) { | |
| putShortUnaligned(o, offset, (short)x); | ||
| } | ||
| /** @see #putLongUnaligned(Object, long, long, boolean) */ | ||
| @ForceInline | ||
| public final void putCharUnaligned(Object o, long offset, char x, boolean bigEndian) { | ||
| putCharUnaligned(o, offset, convEndian(bigEndian, x)); | ||
| } | ||
|
|
@@ -3829,10 +3837,10 @@ private void putShortParts(Object o, long offset, byte i0, byte i1) { | |
| private static long toUnsignedLong(int n) { return n & 0xffffffffl; } | ||
|
|
||
| // Maybe byte-reverse an integer | ||
| private static char convEndian(boolean big, char n) { return big == BIG_ENDIAN ? n : Character.reverseBytes(n); } | ||
| private static short convEndian(boolean big, short n) { return big == BIG_ENDIAN ? n : Short.reverseBytes(n) ; } | ||
| private static int convEndian(boolean big, int n) { return big == BIG_ENDIAN ? n : Integer.reverseBytes(n) ; } | ||
| private static long convEndian(boolean big, long n) { return big == BIG_ENDIAN ? n : Long.reverseBytes(n) ; } | ||
| @ForceInline private static char convEndian(boolean big, char n) { return big == BIG_ENDIAN ? n : Character.reverseBytes(n); } | ||
| @ForceInline private static short convEndian(boolean big, short n) { return big == BIG_ENDIAN ? n : Short.reverseBytes(n) ; } | ||
| @ForceInline private static int convEndian(boolean big, int n) { return big == BIG_ENDIAN ? n : Integer.reverseBytes(n) ; } | ||
| @ForceInline private static long convEndian(boolean big, long n) { return big == BIG_ENDIAN ? n : Long.reverseBytes(n) ; } | ||
|
|
||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the
ensureNativeAccesscode only depend on this one? Also, I'm having an hard time thinking that C2 can't inline this simple method? Isn't this an "accessor" ?