This is an automated email from the ASF dual-hosted git repository. bchapuis pushed a commit to branch data-refactoring in repository https://gitbox.apache.org/repos/asf/incubator-baremaps.git
commit e580db8e5d2d515cb715776af4150f8f58390315 Author: Bertil Chapuis <[email protected]> AuthorDate: Sat Apr 5 00:45:11 2025 +0200 Improve resource management and javadoc --- .../org/apache/baremaps/data/memory/Memory.java | 111 +++++++++++++++------ .../baremaps/data/memory/MemoryException.java | 18 ++-- .../data/memory/MemoryMappedDirectory.java | 44 ++++---- .../baremaps/data/memory/MemoryMappedFile.java | 42 +++++--- .../apache/baremaps/data/memory/OffHeapMemory.java | 32 +++--- .../apache/baremaps/data/memory/OnHeapMemory.java | 31 +++--- 6 files changed, 182 insertions(+), 96 deletions(-) diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/Memory.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/Memory.java index d0a2580bf..5c3e39dc9 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/Memory.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/Memory.java @@ -17,15 +17,13 @@ package org.apache.baremaps.data.memory; - - import java.io.Closeable; import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; -/** A base class to manage segments of on-heap, off-heap, or on-disk memory. */ +/** A base class for managing on-heap, off-heap, or memory-mapped segments. */ public abstract class Memory<T extends ByteBuffer> implements Closeable { private final int segmentSize; @@ -36,10 +34,14 @@ public abstract class Memory<T extends ByteBuffer> implements Closeable { protected final List<T> segments = new ArrayList<>(); + // Flag to track if this Memory has been closed + protected volatile boolean closed = false; + /** - * Constructs a memory with a given segment size. + * Constructs a memory with the specified segment size. * - * @param segmentSize the size of the segments + * @param segmentSize the size of the segments (must be a power of 2) + * @throws IllegalArgumentException if the segment size is not a power of 2 */ protected Memory(int segmentSize) { if ((segmentSize & -segmentSize) != segmentSize) { @@ -51,37 +53,38 @@ public abstract class Memory<T extends ByteBuffer> implements Closeable { } /** - * Returns the size of the segments. + * Returns the size of each segment. * - * @return the size of the segments + * @return the segment size in bytes */ public int segmentSize() { return segmentSize; } /** - * Returns the bit shift to find a segment index from a memory position. + * Returns the bit shift used to find a segment index from a memory position. * - * @return the bit shift + * @return the bit shift value */ public long segmentShift() { return segmentShift; } /** - * Returns the bit mask to find a segment offset from a memory position. + * Returns the bit mask used to find an offset within a segment from a memory position. * - * @return the bit mask + * @return the bit mask value */ public long segmentMask() { return segmentMask; } /** - * Returns a segment of the memory. + * Returns a segment at the specified index, allocating it if necessary. * - * @param index the index of the segment - * @return the segment + * @param index the segment index + * @return the segment as a ByteBuffer + * @throws MemoryException if segment allocation fails */ public ByteBuffer segment(int index) { if (segments.size() <= index) { @@ -94,38 +97,82 @@ public abstract class Memory<T extends ByteBuffer> implements Closeable { return segment; } - /** The allocation of segments is synchronized to enable access by multiple threads. */ - private synchronized ByteBuffer allocate(int index) { - while (segments.size() <= index) { - segments.add(null); + /** + * Checks if this Memory has been closed. + * + * @throws IllegalStateException if this Memory has been closed + */ + protected void checkNotClosed() { + if (closed) { + throw new IllegalStateException("Memory has been closed"); } - T segment = segments.get(index); - if (segment == null) { - segment = allocate(index, segmentSize); - segments.set(index, segment); + } + + /** + * Allocates a segment at the specified index. Thread-safe method. + * + * @param index the segment index + * @return the allocated segment + * @throws MemoryException if allocation fails + */ + private synchronized ByteBuffer allocate(int index) { + checkNotClosed(); + + try { + while (segments.size() <= index) { + segments.add(null); + } + T segment = segments.get(index); + if (segment == null) { + segment = allocate(index, segmentSize); + segments.set(index, segment); + } + return segment; + } catch (OutOfMemoryError e) { + throw new MemoryException( + "Failed to allocate memory segment of size " + segmentSize + " bytes", e); + } catch (Exception e) { + throw new MemoryException("Failed to allocate memory segment", e); } - return segment; } - /** Returns the size of the allocated memory. */ + /** + * Returns the total size of allocated memory. + * + * @return the size in bytes + */ public long size() { + checkNotClosed(); return (long) segments.size() * (long) segmentSize; } /** - * Allocates a segment for a given index and size. + * Allocates a segment of the specified size. * - * @param index the index of the segment - * @param size the size of the segment - * @return the segment + * @param index the segment index + * @param size the segment size in bytes + * @return the allocated segment */ protected abstract T allocate(int index, int size); /** - * Clears the memory and the underlying resources. + * Releases resources associated with this memory. + * Unlike {@link #clear()}, this method does not delete underlying data. + * + * @throws IOException if an I/O error occurs */ - public abstract void clear() throws IOException; - - + @Override + public synchronized void close() throws IOException { + if (!closed) { + closed = true; + } + } + /** + * Deletes all data managed by this memory. + * Unlike {@link #close()}, this method removes underlying data. + * + * @throws IOException if an I/O error occurs + */ + public abstract void clear() throws IOException; } diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryException.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryException.java index 3a4cbb6e0..e261269f2 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryException.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryException.java @@ -17,35 +17,35 @@ package org.apache.baremaps.data.memory; -/** Signals that an exception occurred in a memory. */ +/** Exception thrown when a memory operation fails. */ public class MemoryException extends RuntimeException { - /** Constructs a {@link MemoryException} with {@code null} as its error detail message. */ + /** Constructs a MemoryException with no message or cause. */ public MemoryException() {} /** - * Constructs an {@link MemoryException} with the specified detail message. + * Constructs a MemoryException with the specified message. * - * @param message the message + * @param message error message */ public MemoryException(String message) { super(message); } /** - * Constructs a {@link MemoryException} with the specified cause. + * Constructs a MemoryException with the specified cause. * - * @param cause the cause + * @param cause the underlying cause */ public MemoryException(Throwable cause) { super(cause); } /** - * Constructs a {@link MemoryException} with the specified detail message and cause. + * Constructs a MemoryException with the specified message and cause. * - * @param message the message - * @param cause the cause + * @param message error message + * @param cause the underlying cause */ public MemoryException(String message, Throwable cause) { super(message, cause); diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedDirectory.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedDirectory.java index 82d2e0af7..37a7a9edf 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedDirectory.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedDirectory.java @@ -17,8 +17,6 @@ package org.apache.baremaps.data.memory; - - import java.io.IOException; import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; @@ -29,28 +27,25 @@ import org.apache.baremaps.data.util.FileUtils; import org.apache.baremaps.data.util.MappedByteBufferUtils; -/** - * A {@link Memory} that stores segments on-disk using mapped byte buffers in a directory of files. - */ +/** Memory implementation that uses memory-mapped files in a directory for storage. */ public class MemoryMappedDirectory extends Memory<MappedByteBuffer> { private final Path directory; /** - * Constructs an {@link MemoryMappedDirectory} with a custom directory and a default segment size - * of 1gb. + * Constructs a MemoryMappedDirectory with the specified directory and 1GB segment size. * - * @param directory the directory that stores the data + * @param directory the directory to store segments in */ public MemoryMappedDirectory(Path directory) { this(directory, 1 << 30); } /** - * Constructs an {@link MemoryMappedDirectory} with a custom directory and a custom segment size. + * Constructs a MemoryMappedDirectory with the specified directory and segment size. * - * @param directory the directory that stores the data - * @param segmentBytes the size of the segments in bytes + * @param directory the directory to store segments in + * @param segmentBytes the size of each segment in bytes */ public MemoryMappedDirectory(Path directory, int segmentBytes) { super(segmentBytes); @@ -71,20 +66,35 @@ public class MemoryMappedDirectory extends Memory<MappedByteBuffer> { } } - /** {@inheritDoc} */ + /** + * Releases all mapped buffers but keeps the files intact. + * + * {@inheritDoc} + */ @Override - public void close() throws IOException { + public synchronized void close() throws IOException { + // Release MappedByteBuffer resources for (MappedByteBuffer buffer : segments) { - MappedByteBufferUtils.unmap(buffer); + if (buffer != null) { + MappedByteBufferUtils.unmap(buffer); + } } } - /** {@inheritDoc} */ + /** + * Unmaps all buffers and deletes the directory with all its files. + * + * {@inheritDoc} + */ @Override - public void clear() throws IOException { + public synchronized void clear() throws IOException { + // Release resources first close(); + + // Clear the segment list segments.clear(); + + // Delete the directory and all files in it FileUtils.deleteRecursively(directory); } - } diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedFile.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedFile.java index 5db613d34..be863a33d 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedFile.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/MemoryMappedFile.java @@ -17,8 +17,6 @@ package org.apache.baremaps.data.memory; - - import java.io.IOException; import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; @@ -28,25 +26,25 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import org.apache.baremaps.data.util.MappedByteBufferUtils; -/** A {@link Memory} that stores segments on-disk using mapped byte buffers in a file. */ +/** Memory implementation that uses memory-mapped file for storage. */ public class MemoryMappedFile extends Memory<MappedByteBuffer> { private final Path file; /** - * Constructs an {@link MemoryMappedFile} with a custom file and a default segment size of 1gb. + * Constructs a MemoryMappedFile with the specified file and 1GB segment size. * - * @param file the file that stores the data + * @param file the file to memory-map */ public MemoryMappedFile(Path file) { this(file, 1 << 30); } /** - * Constructs an {@link MemoryMappedFile} with a custom file and a custom segment size. + * Constructs a MemoryMappedFile with the specified file and segment size. * - * @param file the file that stores the data - * @param segmentBytes the size of the segments in bytes + * @param file the file to memory-map + * @param segmentBytes the size of each segment in bytes */ public MemoryMappedFile(Path file, int segmentBytes) { super(segmentBytes); @@ -66,19 +64,35 @@ public class MemoryMappedFile extends Memory<MappedByteBuffer> { } } - /** {@inheritDoc} */ + /** + * Releases all mapped buffers but keeps the file intact. + * + * {@inheritDoc} + */ @Override - public void close() throws IOException { + public synchronized void close() throws IOException { + // Release MappedByteBuffer resources for (MappedByteBuffer buffer : segments) { - MappedByteBufferUtils.unmap(buffer); + if (buffer != null) { + MappedByteBufferUtils.unmap(buffer); + } } } - /** {@inheritDoc} */ + /** + * Unmaps all buffers and deletes the file. + * + * {@inheritDoc} + */ @Override - public void clear() throws IOException { + public synchronized void clear() throws IOException { + // Release resources first close(); + + // Clear the segment list segments.clear(); - Files.delete(file); + + // Delete the file + Files.deleteIfExists(file); } } diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OffHeapMemory.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OffHeapMemory.java index f95f03b51..66a2b7f2e 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OffHeapMemory.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OffHeapMemory.java @@ -17,23 +17,21 @@ package org.apache.baremaps.data.memory; - - import java.io.IOException; import java.nio.ByteBuffer; -/** A {@link Memory} that stores segments off-heap using direct byte buffers. */ +/** Memory implementation that uses off-heap (direct) byte buffers for storage. */ public class OffHeapMemory extends Memory<ByteBuffer> { - /** Constructs an {@link OffHeapMemory} with a default segment size of 1mb. */ + /** Constructs an OffHeapMemory with a 1MB default segment size. */ public OffHeapMemory() { this(1 << 20); } /** - * Constructs an {@link OffHeapMemory} with a custom segment size. + * Constructs an OffHeapMemory with the specified segment size. * - * @param segmentSize the size of the segments in bytes + * @param segmentSize the size of each segment in bytes */ public OffHeapMemory(int segmentSize) { super(segmentSize); @@ -45,15 +43,25 @@ public class OffHeapMemory extends Memory<ByteBuffer> { return ByteBuffer.allocateDirect(size); } - /** {@inheritDoc} */ + /** + * Direct buffers are managed by GC and can't be explicitly freed in Java 8. + * + * {@inheritDoc} + */ @Override - public void close() throws IOException { - // Nothing to close + public synchronized void close() throws IOException { + // Direct ByteBuffers are managed by GC - we can't explicitly free them in Java 8 + // In Java 9+, we could use Cleaner or Unsafe API if needed } - /** {@inheritDoc} */ + /** + * Clears all segments to allow garbage collection. + * + * {@inheritDoc} + */ @Override - public void clear() throws IOException { - // Nothing to clean + public synchronized void clear() throws IOException { + // Clear the segment list to allow GC to reclaim the memory + segments.clear(); } } diff --git a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OnHeapMemory.java b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OnHeapMemory.java index 4b0fff757..d0be3968e 100644 --- a/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OnHeapMemory.java +++ b/baremaps-data/src/main/java/org/apache/baremaps/data/memory/OnHeapMemory.java @@ -17,23 +17,21 @@ package org.apache.baremaps.data.memory; - - import java.io.IOException; import java.nio.ByteBuffer; -/** A {@link Memory} that stores segments on-heap using regular byte buffers. */ +/** Memory implementation that uses on-heap byte buffers for storage. */ public class OnHeapMemory extends Memory<ByteBuffer> { - /** Constructs an {@link OnHeapMemory} with a default segment size of 1mb. */ + /** Constructs an OnHeapMemory with a 1MB default segment size. */ public OnHeapMemory() { this(1 << 20); } /** - * Constructs an {@link OnHeapMemory} with a custom segment size. + * Constructs an OnHeapMemory with the specified segment size. * - * @param segmentSize the size of the segments in bytes + * @param segmentSize the size of each segment in bytes */ public OnHeapMemory(int segmentSize) { super(segmentSize); @@ -45,15 +43,24 @@ public class OnHeapMemory extends Memory<ByteBuffer> { return ByteBuffer.allocate(size); } - /** {@inheritDoc} */ + /** + * On-heap buffers don't require explicit cleanup. + * + * {@inheritDoc} + */ @Override - public void close() throws IOException { - // Nothing to close + public synchronized void close() throws IOException { + // On-heap ByteBuffers don't need special cleanup } - /** {@inheritDoc} */ + /** + * Clears all segments to allow garbage collection. + * + * {@inheritDoc} + */ @Override - public void clear() throws IOException { - // Nothing to clean + public synchronized void clear() throws IOException { + // Clear the segment list to allow GC to reclaim the memory + segments.clear(); } }
