Merge pull request #249 from apache/changes_in_main_tree

Changes to correct issues in 5.0.0

Author: leerho

pom.xml

@@ -34,7 +34,7 @@ under the License.
 
   <groupId>org.apache.datasketches</groupId>
   <artifactId>datasketches-memory</artifactId>
-  <version>5.0.0-SNAPSHOT</version>
+  <version>5.1.0-SNAPSHOT</version>
   <packaging>jar</packaging>
 
   <name>${project.artifactId}</name>

src/main/java/org/apache/datasketches/memory/DefaultMemoryRequestServer.java

@@ -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(); }
   }
 
 }

src/main/java/org/apache/datasketches/memory/MemoryRequestServer.java

@@ -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.
@@ -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);
 
 }

src/main/java/org/apache/datasketches/memory/Resource.java

@@ -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;
@@ -32,7 +30,7 @@
  *
  * @author Lee Rhodes
  */
-public interface Resource extends AutoCloseable {
+public interface Resource {
 
   //MemoryRequestServer logic
 
@@ -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>.
@@ -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
@@ -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.
@@ -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.

src/main/java/org/apache/datasketches/memory/WritableMemory.java

@@ -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 ≥ 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);
   }
 
   /**
@@ -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.
@@ -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);
@@ -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.