Skip to content
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.

Sign up for GitHub

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

8292174: Add unaligned layouts to ValueLayout #705

Closed
wants to merge 7 commits into from
+262 −84
Closed

8292174: Add unaligned layouts to ValueLayout #705

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
968e0f8
Add unaligned value layouts
minborg Aug 10, 2022
a667b61
Update after PR comments
minborg Aug 11, 2022
a3ac209
Fix test and remove unaligned of size 8 bits
minborg Aug 11, 2022
e0e6e5a
Remove duplicate use of UNALIGNED
minborg Aug 12, 2022
ab34e8a
remove comment
minborg Aug 15, 2022
4fc3d16
Change unaligned init code
minborg Aug 16, 2022
b82bf57
Fix JavaDocs in ValueLayout
minborg Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/java.base/share/classes/java/lang/foreign/ValueLayout.java
View file
Original file line number Diff line number Diff line change
@@ -649,6 +649,8 @@ public boolean isUnbounded() {
}
}

// Some constants below are accessed from generated bytecode, see BindingSpecializer::emitLoadLayoutConstant
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which constants "below" ? The comment would have been perfect in the old code, but now it seems the constants have been removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The constants JAVA_BYTE. JAVA_BOOLEAN and JAVA_*_UNALIGNED are used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, before they were all in a special class (Platform) but now the public layouts are used.

Copy link
Collaborator

@mcimadamore mcimadamore Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What I meant is: the comment you copied referred to the layout definitions which have now been removed. So I think it's better to remove the comment.

E.g. I believe the comment was meant to say: these layout constants are unused in Java code, but they are used in some of the code generated by the binding specializer (so please ignore the fact that the IDE is telling you these constants are unused).

But now we no longer need that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK. I thought the comment meant: "Do not change, move or remove any of these constants as they are invisibly used in bytecode generated in BindingSpecializer.". But I can remove the comment.


/**
* A value layout constant whose size is the same as that of a machine address ({@code size_t}),
* bit alignment set to {@code sizeof(size_t) * 8}, byte order set to {@link ByteOrder#nativeOrder()}.
24 changes: 3 additions & 21 deletions src/java.base/share/classes/jdk/internal/foreign/abi/BindingSpecializer.java
View file
Original file line number Diff line number Diff line change
@@ -727,15 +727,15 @@ private void emitAllocateCall(long size, long alignment) {
private Class<?> emitLoadLayoutConstant(Class<?> type) {
Class<?> valueLayoutType = valueLayoutTypeFor(type);
String valueLayoutConstantName = valueLayoutConstantFor(type);
emitGetStatic(BindingSpecializer.Runtime.class, valueLayoutConstantName, valueLayoutType.descriptorString());
emitGetStatic(ValueLayout.class, valueLayoutConstantName, valueLayoutType.descriptorString());
return valueLayoutType;
}

private static String valueLayoutConstantFor(Class<?> type) {
if (type == boolean.class) {
return "JAVA_BOOLEAN_UNALIGNED";
return "JAVA_BOOLEAN";
} else if (type == byte.class) {
return "JAVA_BYTE_UNALIGNED";
return "JAVA_BYTE";
} else if (type == short.class) {
return "JAVA_SHORT_UNALIGNED";
} else if (type == char.class) {
@@ -917,22 +917,4 @@ private void emitReturn(Class<?> type) {
mv.visitInsn(opcode);
}

// constants that are accessed from the generated bytecode
// see emitLoadLayoutConstant
static final class Runtime {

private Runtime() {
}

// unaligned constants
static final ValueLayout.OfBoolean JAVA_BOOLEAN_UNALIGNED = JAVA_BOOLEAN;
static final ValueLayout.OfByte JAVA_BYTE_UNALIGNED = JAVA_BYTE;
static final ValueLayout.OfShort JAVA_SHORT_UNALIGNED = JAVA_SHORT.withBitAlignment(8);
static final ValueLayout.OfChar JAVA_CHAR_UNALIGNED = JAVA_CHAR.withBitAlignment(8);
static final ValueLayout.OfInt JAVA_INT_UNALIGNED = JAVA_INT.withBitAlignment(8);
static final ValueLayout.OfLong JAVA_LONG_UNALIGNED = JAVA_LONG.withBitAlignment(8);
static final ValueLayout.OfFloat JAVA_FLOAT_UNALIGNED = JAVA_FLOAT.withBitAlignment(8);
static final ValueLayout.OfDouble JAVA_DOUBLE_UNALIGNED = JAVA_DOUBLE.withBitAlignment(8);
static final ValueLayout.OfAddress ADDRESS_UNALIGNED = ADDRESS.withBitAlignment(8);
}
}
8 changes: 0 additions & 8 deletions src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java
View file
Original file line number Diff line number Diff line change
@@ -507,14 +507,6 @@ static Object read(MemorySegment ptr, Class<?> type) {
}
}

// unaligned constants
public final static ValueLayout.OfShort JAVA_SHORT_UNALIGNED = JAVA_SHORT.withBitAlignment(8);
public final static ValueLayout.OfChar JAVA_CHAR_UNALIGNED = JAVA_CHAR.withBitAlignment(8);
public final static ValueLayout.OfInt JAVA_INT_UNALIGNED = JAVA_INT.withBitAlignment(8);
public final static ValueLayout.OfLong JAVA_LONG_UNALIGNED = JAVA_LONG.withBitAlignment(8);
public final static ValueLayout.OfFloat JAVA_FLOAT_UNALIGNED = JAVA_FLOAT.withBitAlignment(8);
public final static ValueLayout.OfDouble JAVA_DOUBLE_UNALIGNED = JAVA_DOUBLE.withBitAlignment(8);

public static MethodType inferMethodType(FunctionDescriptor descriptor) {
MethodType type = MethodType.methodType(descriptor.returnLayout().isPresent() ?
carrierFor(descriptor.returnLayout().get()) : void.class);
17 changes: 1 addition & 16 deletions test/jdk/java/foreign/TestMemoryDereference.java
View file
Original file line number Diff line number Diff line change
@@ -35,14 +35,7 @@
import java.lang.foreign.ValueLayout;
import org.testng.annotations.*;

import static java.lang.foreign.ValueLayout.ADDRESS;
import static java.lang.foreign.ValueLayout.JAVA_BOOLEAN;
import static java.lang.foreign.ValueLayout.JAVA_BYTE;
import static java.lang.foreign.ValueLayout.JAVA_CHAR;
import static java.lang.foreign.ValueLayout.JAVA_DOUBLE;
import static java.lang.foreign.ValueLayout.JAVA_FLOAT;
import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.JAVA_SHORT;
import static java.lang.foreign.ValueLayout.*;
import static org.testng.Assert.*;

public class TestMemoryDereference {
@@ -97,14 +90,6 @@ <Z> Accessor<Z> of(Z value,
}
}

// unaligned constants
public final static ValueLayout.OfShort JAVA_SHORT_UNALIGNED = JAVA_SHORT.withBitAlignment(8);
public final static ValueLayout.OfChar JAVA_CHAR_UNALIGNED = JAVA_CHAR.withBitAlignment(8);
public final static ValueLayout.OfInt JAVA_INT_UNALIGNED = JAVA_INT.withBitAlignment(8);
public final static ValueLayout.OfFloat JAVA_FLOAT_UNALIGNED = JAVA_FLOAT.withBitAlignment(8);
public final static ValueLayout.OfDouble JAVA_DOUBLE_UNALIGNED = JAVA_DOUBLE.withBitAlignment(8);
public final static ValueLayout.OfAddress ADDRESS_UNALIGNED = ADDRESS.withBitAlignment(8);

@Test(dataProvider = "accessors")
public void testMemoryAccess(String testName, Accessor<?> accessor) {
accessor.test();
16 changes: 1 addition & 15 deletions test/micro/org/openjdk/bench/java/lang/foreign/JavaLayouts.java
View file
Original file line number Diff line number Diff line change
@@ -26,29 +26,15 @@
import java.lang.foreign.ValueLayout;
import java.lang.invoke.VarHandle;

import static java.lang.foreign.ValueLayout.JAVA_FLOAT;
import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.JAVA_LONG;
import static java.lang.foreign.ValueLayout.*;

/**
* Some useful Java {@link ValueLayout} and associated {@link ValueLayout#arrayElementVarHandle(int...)} var handles.
*/
public class JavaLayouts {
static final ValueLayout.OfInt JAVA_INT_UNALIGNED = JAVA_INT.withBitAlignment(8);

static final ValueLayout.OfFloat JAVA_FLOAT_UNALIGNED = JAVA_FLOAT.withBitAlignment(8);

static final ValueLayout.OfLong JAVA_LONG_UNALIGNED = JAVA_LONG.withBitAlignment(8);

static final VarHandle VH_INT_UNALIGNED = JAVA_INT_UNALIGNED.arrayElementVarHandle();

static final VarHandle VH_INT = JAVA_INT.arrayElementVarHandle();

static final VarHandle VH_FLOAT_UNALIGNED = JAVA_FLOAT_UNALIGNED.arrayElementVarHandle();

static final VarHandle VH_FLOAT = JAVA_FLOAT.arrayElementVarHandle();

static final VarHandle VH_LONG_UNALIGNED = JAVA_LONG_UNALIGNED.arrayElementVarHandle();

static final VarHandle VH_LONG = JAVA_LONG.arrayElementVarHandle();
}
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverConstant.java
View file
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@
import java.nio.ByteOrder;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNew.java
View file
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@
import java.nio.ByteOrder;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNewHeap.java
View file
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@
import java.nio.IntBuffer;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstant.java
View file
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@
import java.nio.ByteOrder;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
5 changes: 1 addition & 4 deletions test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstantHeap.java
View file
Original file line number Diff line number Diff line change
@@ -41,10 +41,7 @@
import java.nio.ByteOrder;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_BYTE;
import static java.lang.foreign.ValueLayout.JAVA_DOUBLE;
import static java.lang.foreign.ValueLayout.JAVA_FLOAT;
import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverNonConstantMapped.java
View file
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@
import java.nio.file.StandardOpenOption;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
2 changes: 1 addition & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/LoopOverPollutedSegments.java
View file
Original file line number Diff line number Diff line change
@@ -38,7 +38,7 @@

import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_INT;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
7 changes: 6 additions & 1 deletion test/micro/org/openjdk/bench/java/lang/foreign/UnrolledAccess.java
View file
Original file line number Diff line number Diff line change
@@ -24,14 +24,15 @@
package org.openjdk.bench.java.lang.foreign;

import java.lang.foreign.*;
import java.lang.invoke.VarHandle;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.options.Options;
import org.openjdk.jmh.runner.options.OptionsBuilder;
import sun.misc.Unsafe;
import java.util.concurrent.TimeUnit;

import static java.lang.foreign.ValueLayout.JAVA_LONG;
import static java.lang.foreign.ValueLayout.*;

@BenchmarkMode(Mode.AverageTime)
@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
@@ -43,6 +44,10 @@ public class UnrolledAccess extends JavaLayouts {

static final Unsafe U = Utils.unsafe;

static final VarHandle VH_LONG_UNALIGNED = JAVA_LONG_UNALIGNED.arrayElementVarHandle();

static final VarHandle VH_LONG = JAVA_LONG.arrayElementVarHandle();

final static int SIZE = 1024;

@State(Scope.Benchmark)