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

Changes to correct issues in 5.0.0 #249

Merged
merged 6 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ under the License.

<groupId>org.apache.datasketches</groupId>
<artifactId>datasketches-memory</artifactId>
<version>5.0.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

the main branch should be 5.1.0-SNAPSHOT

<version>5.1.0-SNAPSHOT</version>
<packaging>jar</packaging>

<name>${project.artifactId}</name>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,72 +24,42 @@

/**
* This example MemoryRequestServer is simple but demonstrates one of many ways to
* manage continuous requests for larger memory.
* manage continuous requests for larger or smaller memory.
* This capability is only available for writable, non-file-memory-mapping resources.
*
* @author Lee Rhodes
*/
public final class DefaultMemoryRequestServer implements MemoryRequestServer {
private final boolean offHeap; //create the new memory off-heap; otherwise, on-heap
private final boolean copyOldToNew; //copy data from old memory to new memory.

/**
* Default constructor.
* Create new memory on-heap and do not copy old contents to new memory.
*/
public DefaultMemoryRequestServer() {
this(false, false);
}

/**
* Constructor with parameters
* @param offHeap if true, the returned new memory will be off heap
* @param copyOldToNew if true, the data from the current memory will be copied to the new memory,
* starting at address 0, and through the currentMemory capacity.
*/
public DefaultMemoryRequestServer(
final boolean offHeap,
final boolean copyOldToNew) {
this.offHeap = offHeap;
this.copyOldToNew = copyOldToNew;
}
public DefaultMemoryRequestServer() { }

@Override
public WritableMemory request(
final WritableMemory currentWmem,
final long newCapacityBytes,
final long alignmentBytes,
final ByteOrder byteOrder,
final Arena arena) {
final ByteOrder order = currentWmem.getTypeByteOrder();
final long currentBytes = currentWmem.getCapacity();
final WritableMemory newWmem;

if (newCapacityBytes <= currentBytes) {
throw new IllegalArgumentException("newCapacityBytes must be &gt; currentBytes");
}

if (offHeap) {
newWmem = WritableMemory.allocateDirect(newCapacityBytes, 8, order, this, arena);
if (arena != null) {
newWmem = WritableMemory.allocateDirect(newCapacityBytes, alignmentBytes, byteOrder, this, arena);
}
else { //On-heap
if (newCapacityBytes > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Requested capacity exceeds Integer.MAX_VALUE.");
}
newWmem = WritableMemory.allocate((int)newCapacityBytes, order, this);
}

if (copyOldToNew) {
currentWmem.copyTo(0, newWmem, 0, currentBytes);
newWmem = WritableMemory.allocate((int)newCapacityBytes, byteOrder, this);
}

return newWmem;
}

@Override
public void requestClose(
final WritableMemory memToClose,
final WritableMemory newMemory) {
//try to make this operation idempotent.
if (memToClose.isCloseable()) { memToClose.close(); }
public void requestClose(final Arena arena) {
if (arena.scope().isAlive()) { arena.close(); }
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
package org.apache.datasketches.memory;

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

/**
* The MemoryRequestServer is a callback interface to provide a means to request more memory
* The MemoryRequestServer is a callback interface to provide a means to request more or less memory
* for heap and off-heap WritableMemory resources that are not file-memory-mapped backed resources.
*
* <p>Note: this only works with Java 21.
Expand All @@ -32,53 +33,26 @@
public interface MemoryRequestServer {

/**
* Request new WritableMemory with the given newCapacityBytes. The current WritableMemory can be used to
* determine the byte order of the returned WritableMemory and other properties. A new confined Arena is
* assigned.
* @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.
* @return new WritableMemory with the requested capacity.
*/
default WritableMemory request(
WritableMemory currentWritableMemory,
long newCapacityBytes) {

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 arena the Arena to be used for the newly allocated memory. It must be non-null.
* Request new WritableMemory with the given newCapacityBytes.
* @param newCapacityBytes The capacity being requested.
* @param alignmentBytes requested segment alignment. Typically 1, 2, 4 or 8.
* @param byteOrder the given <i>ByteOrder</i>. It must be non-null.
* @param arena the given arena to manage the new off-heap WritableMemory.
* If arena is null, the requested WritableMemory will be off-heap.
* Warning: This class is not thread-safe. Specifying an Arena that allows multiple threads is not recommended.
* @return new WritableMemory with the requested capacity.
*/
WritableMemory request(
WritableMemory currentWritableMemory,
long newCapacityBytes,
long alignmentBytes,
ByteOrder byteOrder,
Arena arena);

/**
* Request to close the resource, if applicable.
*
* @param memToClose the relevant WritableMemory to be considered for closing. It must be non-null.
*/
default void requestClose(WritableMemory memToClose) {
requestClose(memToClose, null);
}

/**
* Request to close the resource, if applicable.
*
* @param memToClose the relevant WritableMemory to be considered for closing. It must be non-null.
* @param newMemory the newly allocated WritableMemory.
* The newMemory reference is returned from the client for the convenience of the system that
* owns the responsibility of memory allocation. It may be null.
* Request to close the area managing all the related resources, if applicable.
* Be careful when you request to close the given Arena, you may be closing other resources as well.
* @param arena the given arena to use to close all its managed resources.
*/
void requestClose(
WritableMemory memToClose,
WritableMemory newMemory);
void requestClose( Arena arena);

}
54 changes: 15 additions & 39 deletions src/main/java/org/apache/datasketches/memory/Resource.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
package org.apache.datasketches.memory;

import java.lang.foreign.Arena;
import java.lang.foreign.FunctionDescriptor;
import java.lang.foreign.Linker.Option;
import java.lang.foreign.MemorySegment;
import java.lang.foreign.MemorySegment.Scope;
import java.nio.ByteBuffer;
Expand All @@ -32,7 +30,7 @@
*
* @author Lee Rhodes
*/
public interface Resource extends AutoCloseable {
public interface Resource {

//MemoryRequestServer logic

Expand Down Expand Up @@ -92,32 +90,6 @@ public interface Resource extends AutoCloseable {
*/
ByteBuffer asByteBufferView(ByteOrder order);

/**
* <i>From Java 21 java.lang.foreign.Arena::close():</i>
* Closes this arena. If this method completes normally, the arena scope is no longer {@linkplain Scope#isAlive() alive},
* and all the memory segments associated with it can no longer be accessed. Furthermore, any off-heap region of memory backing the
* segments obtained from this arena are also released.
*
* <p>This operation is not idempotent; that is, closing an already closed arena <em>always</em> results in an
* exception being thrown. This reflects a deliberate design choice: failure to close an arena might reveal a bug
* in the underlying application logic.</p>
*
* <p>If this method completes normally, then {@code java.lang.foreign.Arena.scope().isAlive() == false}.
* Implementations are allowed to throw {@link UnsupportedOperationException} if an explicit close operation is
* not supported.</p>
*
* @see java.lang.foreign.MemorySegment.Scope#isAlive()
*
* @throws IllegalStateException if the arena has already been closed.
* @throws IllegalStateException if a segment associated with this arena is being accessed concurrently, e.g.
* by a {@linkplain java.lang.foreign.Linker#downcallHandle(FunctionDescriptor, Option...) downcall method handle}.
* @throws WrongThreadException if this arena is confined, and this method is called from a thread
* other than the arena's owner thread.
* @throws UnsupportedOperationException if this arena cannot be closed explicitly.
*/
@Override
void close();

/**
* Compares the bytes of this Resource to <i>that</i> Resource.
* Returns <i>(this &lt; that) ? (some negative value) : (this &gt; that) ? (some positive value) : 0;</i>.
Expand Down Expand Up @@ -170,6 +142,13 @@ boolean equalTo(
*/
void force();

/**
* Returns the arena used to create this resource and possibly other resources.
* Be careful when you close the returned Arena, you may be closing other resources as well.
* @return the arena used to create this resource and possibly other resources.
*/
Arena getArena();

/**
* Gets the capacity of this object in bytes
* @return the capacity of this object in bytes
Expand Down Expand Up @@ -198,13 +177,6 @@ boolean equalTo(
*/
boolean hasByteBuffer();

/**
* Return true if this resource is likely to be closeable, but not guaranteed.
* There is no way to determine if the specific type of Arena is explicitly closeable.
* @return true if this resource is likely to be closeable.
*/
boolean isCloseable();

/**
* Is the underlying resource scope alive?
* @return true, if the underlying resource scope is alive.
Expand Down Expand Up @@ -346,10 +318,14 @@ boolean equalTo(
ByteBuffer toByteBuffer(ByteOrder order);

/**
* Returns a copy of the underlying MemorySegment.
* @return a copy of the underlying MemorySegment.
* Returns a copy of the underlying MemorySegment in the given arena.
* @param arena the given arena.
* If the desired result is to be off-heap, the arena must not be null.
* Otherwise, the result will be on-heap.
* @param alignment requested segment alignment. Typically 1, 2, 4 or 8.
* @return a copy of the underlying MemorySegment in the given arena.
*/
MemorySegment toMemorySegment();
MemorySegment toMemorySegment(Arena arena, long alignment);

/**
* Returns a brief description of this object.
Expand Down
60 changes: 10 additions & 50 deletions src/main/java/org/apache/datasketches/memory/WritableMemory.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,40 +73,18 @@ static WritableMemory writableWrap(
/**
* Maps the entire given file into native-ordered WritableMemory for write operations with Arena.ofConfined().
* Calling this method is equivalent to calling
* {@link #writableMap(File, long, long, ByteOrder) writableMap(file, 0, file.length(), ByteOrder.nativeOrder())}.
* {@link #writableMap(File, long, long, ByteOrder, Arena) writableMap(file, 0, file.length(), ByteOrder.nativeOrder(), arena)}.
* @param file the given file to map. It must be non-null and writable.
* @param arena the given arena to manage the new off-heap WritableMemory. It must be non-null.
* Warning: This class is not thread-safe. Specifying an Arena that allows multiple threads is not recommended.
* @return a file-mapped WritableMemory
* @throws IllegalArgumentException if file is not readable or not writable.
* @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 WritableMemory writableMap(File file) throws IOException {
return WritableMemoryImpl.wrapMap(file, 0, file.length(), ByteOrder.nativeOrder(), false, Arena.ofConfined());
}

/**
* Maps the specified portion of the given file into Memory for write operations with Arena.ofConfined().
* Calling this method is equivalent to calling
* {@link #writableMap(File, long, long, ByteOrder, Arena)
* writableMap(file, fileOffsetBytes, capacityBytes, ByteOrder, Arena.ofConfined())}.
* @param file the given file to map. It must be non-null with a non-negative length and writable.
* @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 be &ge; 0.
* @param byteOrder the byte order to be used. It must be non-null.
* @return mapped WritableMemory.
* @throws IllegalArgumentException -- if file is not readable or writable.
* @throws IllegalArgumentException -- if file is not writable.
* @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 WritableMemory writableMap(
File file,
long fileOffsetBytes,
long capacityBytes,
ByteOrder byteOrder) throws IOException {
return WritableMemoryImpl.wrapMap(file, fileOffsetBytes, capacityBytes, byteOrder, false, Arena.ofConfined());
static WritableMemory writableMap(File file, Arena arena) throws IOException {
return WritableMemoryImpl.wrapMap(file, 0, file.length(), ByteOrder.nativeOrder(), false, arena);
}

/**
Expand All @@ -115,8 +93,9 @@ static WritableMemory writableMap(
* @param fileOffsetBytes the position in the given file in bytes. It must not be negative.
* @param capacityBytes the size of the mapped Memory.
* @param byteOrder the given <i>ByteOrder</i>. It must be non-null.
* @param arena the given arena to map. It must be non-null.
* @param arena the given arena to manage the new off-heap WritableMemory. It must be non-null.
* Warning: This class is not thread-safe. Specifying an Arena that allows multiple threads is not recommended.
*
* @return a file-mapped WritableMemory.
* @throws IllegalArgumentException if file is not readable or not writable.
* @throws IOException if the specified path does not point to an existing file, or if some other I/O error occurs.
Expand All @@ -139,34 +118,15 @@ static WritableMemory writableMap(
* The allocated memory will be 8-byte aligned.
* Native byte order is assumed.
* A new DefaultMemoryRequestServer() is created.
* A new Arena.ofConfined() is created.
*
* <p><b>NOTE:</b> Native/Direct memory acquired may have garbage in it.
* It is the responsibility of the using application to clear this memory, if required,
* and to call <i>close()</i> when done.</p>
* @param capacityBytes the size of the desired memory in bytes.
* Warning: This class is not thread-safe.
*
* @return WritableMemory for this off-heap, native resource.
*/
static WritableMemory allocateDirect(long capacityBytes) {
return allocateDirect(capacityBytes, 8, ByteOrder.nativeOrder(), new DefaultMemoryRequestServer(), Arena.ofConfined());
}

/**
* Allocates and provides access to capacityBytes directly in native (off-heap) memory.
* The allocated memory will be 8-byte aligned.
* Native byte order is assumed.
* A new DefaultMemoryRequestServer() is created.
*
* <p><b>NOTE:</b> Native/Direct memory acquired may have garbage in it.
* It is the responsibility of the using application to clear this memory, if required,
* and to call <i>close()</i> when done.</p>
* @param capacityBytes the size of the desired memory in bytes.
* @param arena the given arena to use. It must be non-null.
* @param arena the given arena to manage the new off-heap WritableMemory. It must be non-null.
* Warning: This class is not thread-safe. Specifying an Arena that allows multiple threads is not recommended.
*
* @return WritableMemory for this off-heap, native resource.
* @return a WritableMemory for this off-heap resource.
*/
static WritableMemory allocateDirect(long capacityBytes, Arena arena) {
return allocateDirect(capacityBytes, 8, ByteOrder.nativeOrder(), new DefaultMemoryRequestServer(), arena);
Expand All @@ -184,7 +144,7 @@ static WritableMemory allocateDirect(long capacityBytes, Arena arena) {
* @param byteOrder the given <i>ByteOrder</i>. It must be non-null.
* @param memReqSvr A user-specified MemoryRequestServer, which may be null.
* This is a callback mechanism for a user client of direct memory to request more memory.
* @param arena the given arena to use. It must be non-null.
* @param arena the given arena to manage the new off-heap WritableMemory. It must be non-null.
* Warning: This class is not thread-safe. Specifying an Arena that allows multiple threads is not recommended.
*
* @return a WritableMemory for this off-heap resource.
Expand Down
Loading