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.

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

Migrate to Java 21 FFM API #241

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/auto-jdk-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
fail-fast: false

env:
JDK_VERSION: 17
JDK_VERSION: 21

steps:
- name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
Expand All @@ -36,7 +36,7 @@ jobs:
distribution: 'temurin'
java-package: jdk
architecture: x64
java-version: 17
java-version: 21

- name: Cache local Maven repository
uses: actions/cache@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/auto-os-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
fail-fast: false

matrix:
jdk: [ 17 ]
jdk: [ 21 ]
os: [ windows-latest, ubuntu-latest, macos-latest ]
include:
- os: windows-latest
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/javadoc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Setup Java
uses: actions/setup-java@v4
with:
java-version: '17'
java-version: '21'
distribution: 'temurin'

- name: Echo Java Version
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ datasketches-memory*/.gitignore
*.ipr
*.iws

# Netbeans project files
nb-configuration.xml

# Additional tools
.clover/

Expand Down
14 changes: 8 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ under the License.

<!-- System-wide properties -->
<maven.version>3.6.3</maven.version>
<java.version>17</java.version>
<java.version>21</java.version>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
<argLine>-Xmx4g -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 --add-modules=jdk.incubator.foreign</argLine>
<argLine>-Xmx4g -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8 --enable-preview</argLine>
<charset.encoding>UTF-8</charset.encoding>
<project.build.sourceEncoding>${charset.encoding}</project.build.sourceEncoding>
<project.build.resourceEncoding>${charset.encoding}</project.build.resourceEncoding>
Expand Down Expand Up @@ -160,8 +160,9 @@ under the License.
<artifactId>maven-compiler-plugin</artifactId>
<version>${maven-compiler-plugin.version}</version>
<configuration>
<release>21</release>
<compilerArgs>
<arg>--add-modules=jdk.incubator.foreign</arg>
<arg>--enable-preview</arg>
</compilerArgs>
</configuration>
</plugin>
Expand All @@ -187,7 +188,7 @@ under the License.
<configuration>
<rules>
<requireJavaVersion>
<version>[17,18)</version>
<version>21</version>
</requireJavaVersion>
<requireMavenVersion>
<version>[${maven.version},4.0.0)</version>
Expand Down Expand Up @@ -235,7 +236,7 @@ under the License.
<excludePackageNames>org.apache.datasketches.memory/internal</excludePackageNames>
<show>public</show>
<additionalOptions>
<additionalOption>--add-modules=jdk.incubator.foreign</additionalOption>
<additionalOption>--enable-preview</additionalOption>
</additionalOptions>
</configuration>
<executions>
Expand Down Expand Up @@ -281,7 +282,7 @@ under the License.
<artifactId>maven-surefire-plugin</artifactId>
<version>${maven-surefire-failsafe-plugins.version}</version>
<configuration>
<argLine>--add-modules=jdk.incubator.foreign</argLine>
<argLine>--enable-preview</argLine>
<trimStackTrace>false</trimStackTrace>
<useManifestOnlyJar>false</useManifestOnlyJar>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
Expand All @@ -307,6 +308,7 @@ under the License.
<useDefaultExcludes>true</useDefaultExcludes>
<excludes>
<!-- rat uses .gitignore for excludes by default -->
<!-- <exclude>**/nb-configuration.xml</exclude>-->
<exclude>**/*.yaml</exclude>
<exclude>**/*.yml</exclude>
<exclude>**/.*</exclude>
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/apache/datasketches/memory/Buffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.datasketches.memory;

import java.lang.foreign.Arena;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@

package org.apache.datasketches.memory;

import java.lang.foreign.Arena;
import java.nio.ByteOrder;

import jdk.incubator.foreign.ResourceScope;

/**
* This example MemoryRequestServer is simple but demonstrates one of many ways to
* manage continuous requests for larger memory.
Expand Down Expand Up @@ -59,7 +58,7 @@ public DefaultMemoryRequestServer(
public WritableMemory request(
final WritableMemory currentWmem,
final long newCapacityBytes,
final ResourceScope scope) {
final Arena arena) {
final ByteOrder order = currentWmem.getTypeByteOrder();
final long currentBytes = currentWmem.getCapacity();
final WritableMemory newWmem;
Expand All @@ -69,7 +68,7 @@ public WritableMemory request(
}

if (offHeap) {
newWmem = WritableMemory.allocateDirect(newCapacityBytes, 8, scope, order, this);
newWmem = WritableMemory.allocateDirect(arena, newCapacityBytes, 8, order, this);
}
else { //On-heap
if (newCapacityBytes > Integer.MAX_VALUE) {
Expand All @@ -90,7 +89,7 @@ public void requestClose(
final WritableMemory memToClose,
final WritableMemory newMemory) {
//make this operation idempotent.
if (memToClose.isCloseable()) { memToClose.scope().close(); }
if (memToClose.isCloseable()) { memToClose.close(); }
}

}
45 changes: 11 additions & 34 deletions src/main/java/org/apache/datasketches/memory/Memory.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.lang.foreign.Arena;
import java.lang.foreign.MemorySegment;
import java.lang.foreign.MemorySegment.Scope;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Objects;

import org.apache.datasketches.memory.internal.WritableMemoryImpl;

import jdk.incubator.foreign.MemorySegment;
import jdk.incubator.foreign.ResourceScope;

/**
* Defines the read-only API for offset access to a resource.
Expand Down Expand Up @@ -74,8 +76,9 @@ static Memory wrap(
/**
* Maps the given file into <i>Memory</i> for read operations
* Calling this method is equivalent to calling
* {@link #map(File, long, long, ByteOrder)
* {@link #map(Arena, File, long, long, ByteOrder)
* map(file, 0, file.length(), scope, ByteOrder.nativeOrder())}.
* @param arena the given arena. It must be non-null.
* @param file the given file to map. It must be non-null with a non-negative length and readable.
* @return <i>Memory</i> for managing the mapped memory.
* @throws IllegalArgumentException if path is not associated with the default file system.
Expand All @@ -84,12 +87,13 @@ static Memory wrap(
* @throws SecurityException If a security manager is installed and it denies an unspecified permission
* required by the implementation.
*/
static Memory map(File file) throws IOException {
return map(file, 0, file.length(), ByteOrder.nativeOrder());
static Memory map(Arena arena, File file) throws IOException {
return map(arena, file, 0, file.length(), ByteOrder.nativeOrder());
}

/**
* Maps the specified portion of the given file into <i>Memory</i> for read operations.
* @param arena the given arena. It must be non-null.
* @param file the given file to map. It must be non-null,readable and length &ge; 0.
* @param fileOffsetBytes the position in the given file in bytes. It must not be negative.
* @param capacityBytes the size of the mapped memory. It must not be negative..
Expand All @@ -102,39 +106,12 @@ static Memory map(File file) throws IOException {
* required by the implementation.
*/
static Memory map(
Arena arena,
File file,
long fileOffsetBytes,
long capacityBytes,
ByteOrder byteOrder) throws IOException {
final ResourceScope scope = ResourceScope.newConfinedScope();
return WritableMemoryImpl.wrapMap(file, fileOffsetBytes, capacityBytes, scope, true, byteOrder);
}

/**
* Maps the specified portion of the given file into <i>Memory</i> for read operations with a ResourceScope.
* @param file the given file to map. It must be non-null, readable and length &ge; 0.
* @param fileOffsetBytes the position in the given file in bytes. It must not be negative.
* @param capacityBytes the size of the mapped memory. It must not be negative.
* @param scope the given ResourceScope.
* It must be non-null.
* Typically use <i>ResourceScope.newConfinedScope()</i>.
* Warning: specifying a <i>newSharedScope()</i> is not supported.
Copy link
Contributor

@leerho leerho Nov 19, 2024

Choose a reason for hiding this comment

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

In cases where the user supplies the Arena I suggest we add in the javadoc something to the effect:

This Memory component is not thread-safe as it was designed to be used in environments where threading and parallel operation are handled at higher levels. Specifying global(), ofAuto() or ofShared() Arenas could produce unpredictable results

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If Arena.ofConfined() is the only one which makes sense and is applicable here, then perhaps the Arena shouldn't be exposed in the public API at all?

* @param byteOrder the byte order to be used. It must be non-null.
* @return <i>Memory</i> for managing the mapped memory.
* @throws IllegalArgumentException if path is not associated with the default file system.
* @throws IllegalStateException - if scope has been already closed, or if access occurs from a thread other
* than the thread owning scope.
* @throws IOException - if the specified path does not point to an existing file, or if some other I/O error occurs.
* @throws SecurityException - If a security manager is installed and it denies an unspecified permission
* required by the implementation.
*/
static Memory map(
File file,
long fileOffsetBytes,
long capacityBytes,
ResourceScope scope,
ByteOrder byteOrder) throws IOException {
return WritableMemoryImpl.wrapMap(file, fileOffsetBytes, capacityBytes, scope, true, byteOrder);
return WritableMemoryImpl.wrapMap(arena, file, fileOffsetBytes, capacityBytes, true, byteOrder);
}

//NO ALLOCATE DIRECT, makes no sense
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@

package org.apache.datasketches.memory;

import jdk.incubator.foreign.ResourceScope;
import java.lang.foreign.Arena;

/**
* The MemoryRequestServer is a callback interface to provide a means to request more memory
* for heap and off-heap WritableMemory resources that are not file-memory-mapped backed resources.
*
* <p>Note: this only works with Java 17.
* <p>Note: this only works with Java 21.
*
* @author Lee Rhodes
*/
Expand All @@ -42,24 +42,24 @@ public interface MemoryRequestServer {
default WritableMemory request(
WritableMemory currentWritableMemory,
long newCapacityBytes) {
return request(currentWritableMemory, newCapacityBytes, ResourceScope.newConfinedScope());

return request(currentWritableMemory, newCapacityBytes, Arena.ofConfined());
}

/**
* Request new WritableMemory with the given newCapacityBytes. The current Writable Memory can be used to
* determine the byte order of the returned WritableMemory and other properties.
* @param currentWritableMemory the current writableMemory of the client. It must be non-null.
* @param newCapacityBytes The capacity being requested. It must be &gt; the capacity of the currentWritableMemory.
* @param scope the ResourceScope to be used for the newly allocated memory.
* It must be non-null.
* @param arena the Arena to be used for the newly allocated memory. It must be non-null.
* Typically use <i>ResourceScope.newConfinedScope()</i>.
* Warning: specifying a <i>newSharedScope()</i> is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add warning as above. Also replace ResourceScope

* @return new WritableMemory with the requested capacity.
*/
WritableMemory request(
WritableMemory currentWritableMemory,
long newCapacityBytes,
ResourceScope scope);
Arena arena);

/**
* Request to close the resource, if applicable.
Expand Down
26 changes: 14 additions & 12 deletions src/main/java/org/apache/datasketches/memory/MurmurHash3.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

package org.apache.datasketches.memory;

import org.apache.datasketches.memory.internal.MurmurHash3v3;
import java.lang.foreign.MemorySegment;
import org.apache.datasketches.memory.internal.MurmurHash3v4;

import jdk.incubator.foreign.MemorySegment;

/**
* <p>The MurmurHash3 is a fast, non-cryptographic, 128-bit hash function that has
Expand All @@ -39,13 +39,15 @@
* <p>This implementation produces exactly the same hash result as the
* MurmurHash3 function in datasketches-java given compatible inputs.</p>
*
* <p>This version 3 of the implementation leverages the jdk.incubator.foreign package of JDK-17 in place of
* <p>This version 4 of the implementation leverages the java.lang.foreign package of JDK-21 in place of
* the Unsafe class.
*
* @author Lee Rhodes
*/
public final class MurmurHash3 {

private MurmurHash3() { }

//Provided for backward compatibility

/**
Expand All @@ -59,7 +61,7 @@ public final class MurmurHash3 {
public static long[] hash(
final long[] in,
final long seed) {
return MurmurHash3v3.hash(in, seed);
return MurmurHash3v4.hash(in, seed);
}

/**
Expand All @@ -73,7 +75,7 @@ public static long[] hash(
public static long[] hash(
final int[] in,
final long seed) {
return MurmurHash3v3.hash(in, seed);
return MurmurHash3v4.hash(in, seed);
}

/**
Expand All @@ -87,7 +89,7 @@ public static long[] hash(
public static long[] hash(
final char[] in,
final long seed) {
return MurmurHash3v3.hash(in, seed);
return MurmurHash3v4.hash(in, seed);
}

/**
Expand All @@ -101,7 +103,7 @@ public static long[] hash(
public static long[] hash(
final byte[] in,
final long seed) {
return MurmurHash3v3.hash(in, seed);
return MurmurHash3v4.hash(in, seed);
}

//Single primitive inputs
Expand All @@ -118,7 +120,7 @@ public static long[] hash(
final long in,
final long seed,
final long[] hashOut) {
return MurmurHash3v3.hash(in, seed, hashOut);
return MurmurHash3v4.hash(in, seed, hashOut);
}

/**
Expand All @@ -133,7 +135,7 @@ public static long[] hash(
final double in,
final long seed,
final long[] hashOut) {
return MurmurHash3v3.hash(in, seed, hashOut);
return MurmurHash3v4.hash(in, seed, hashOut);
}

/**
Expand All @@ -148,7 +150,7 @@ public static long[] hash(
final String in,
final long seed,
final long[] hashOut) {
return MurmurHash3v3.hash(in, seed, hashOut);
return MurmurHash3v4.hash(in, seed, hashOut);
}

//The main API calls
Expand All @@ -169,7 +171,7 @@ public static long[] hash(
final long lengthBytes,
final long seed,
final long[] hashOut) {
return MurmurHash3v3.hash(mem, offsetBytes, lengthBytes, seed, hashOut);
return MurmurHash3v4.hash(mem, offsetBytes, lengthBytes, seed, hashOut);
}

/**
Expand All @@ -188,7 +190,7 @@ public static long[] hash(
final long lengthBytes,
final long seed,
final long[] hashOut) {
return MurmurHash3v3.hash(seg, offsetBytes, lengthBytes, seed, hashOut);
return MurmurHash3v4.hash(seg, offsetBytes, lengthBytes, seed, hashOut);
}

}
Loading
Loading