leerho commented on code in PR #241:
URL:
https://github.com/apache/datasketches-memory/pull/241#discussion_r1847502818
##########
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:
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 (or disastrous) results
.
--
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]
