frankgrimes97 commented on code in PR #241:
URL:
https://github.com/apache/datasketches-memory/pull/241#discussion_r1850559200
##########
src/main/java/org/apache/datasketches/memory/Memory.java:
##########
@@ -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 ≥ 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.
Review Comment:
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
