You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by "leerho (via GitHub)" <gi...@apache.org> on 2023/02/25 01:40:07 UTC
[GitHub] [datasketches-memory] leerho opened a new pull request, #174: Add javadoc throws
leerho opened a new pull request, #174:
URL: https://github.com/apache/datasketches-memory/pull/174
Hopefully, this is the last of major edits to the core Java8 Memory before I integrate Java 17.
In this group of edits:
- I implemented non-owner thread detection for the methods that create and close memory-mapped files and direct, off-heap memory segments. It would be risky to allow a non-owner thread to close these resources even if it is performed in a synchronized wrapper. This only affects the methods close(), load(), isLoaded(), and force(). This is clearly documented in the concomitant Javadocs.
- I was able to simplify the 16 leaf node classes by moving common variables down to the root ResourceImpl class.
- I removed some other duplication of methods.
- The major task was to completely remove methods that would be incompatible with the Java17 Panama API. This will likely require more tweaking as I merge in the Java 17 Panama code.
--
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: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho commented on a diff in pull request #174: Add javadoc throws
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #174:
URL: https://github.com/apache/datasketches-memory/pull/174#discussion_r1123590668
##########
datasketches-memory-java8/src/main/java/org/apache/datasketches/memory/internal/AllocateDirect.java:
##########
@@ -111,15 +99,15 @@ StepBoolean getValid() {
}
@Override
- public void run() throws MemoryCloseException {
+ public void run() throws IllegalStateException {
deallocate(true);
}
- boolean deallocate(final boolean calledFromCleaner) throws MemoryCloseException {
+ boolean deallocate(final boolean calledFromCleaner) throws IllegalStateException {
if (valid.change()) {
if (calledFromCleaner) {
// Warn about non-deterministic resource cleanup.
- LOG.warning("A WritableHandle was not closed manually");
+ //LOG.warning("A direct resource was not closed explicitly");
Review Comment:
Thinking more, I'll just put it back. I think your questioning its removal is correct.
--
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: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho commented on a diff in pull request #174: Add javadoc throws
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho commented on code in PR #174:
URL: https://github.com/apache/datasketches-memory/pull/174#discussion_r1123581597
##########
datasketches-memory-java8/src/main/java/org/apache/datasketches/memory/internal/AllocateDirect.java:
##########
@@ -111,15 +99,15 @@ StepBoolean getValid() {
}
@Override
- public void run() throws MemoryCloseException {
+ public void run() throws IllegalStateException {
deallocate(true);
}
- boolean deallocate(final boolean calledFromCleaner) throws MemoryCloseException {
+ boolean deallocate(final boolean calledFromCleaner) throws IllegalStateException {
if (valid.change()) {
if (calledFromCleaner) {
// Warn about non-deterministic resource cleanup.
- LOG.warning("A WritableHandle was not closed manually");
+ //LOG.warning("A direct resource was not closed explicitly");
Review Comment:
I debated with myself how useful is this warning. This and the one for mapping are the only uses of Logger in all of Memory. If you feel it is still meaningful, I can put it back.
##########
datasketches-memory-java8/src/main/java/org/apache/datasketches/memory/internal/AllocateDirectWritableMap.java:
##########
@@ -291,15 +274,15 @@ StepBoolean getValid() {
}
@Override
- public void run() throws MemoryCloseException {
+ public void run() throws IllegalStateException {
deallocate(true);
}
- boolean deallocate(final boolean calledFromCleaner) throws MemoryCloseException {
+ boolean deallocate(final boolean calledFromCleaner) throws IllegalStateException {
if (valid.change()) {
if (calledFromCleaner) {
// Warn about non-deterministic resource cleanup.
- LOG.warning("A WritableMapHandleImpl was not closed manually");
+ //LOG.warning("A WritableMapHandleImpl was not closed manually");
Review Comment:
Same comment as the one above.
--
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: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] davecromberge commented on a diff in pull request #174: Add javadoc throws
Posted by "davecromberge (via GitHub)" <gi...@apache.org>.
davecromberge commented on code in PR #174:
URL: https://github.com/apache/datasketches-memory/pull/174#discussion_r1122261111
##########
datasketches-memory-java8/src/main/java/org/apache/datasketches/memory/internal/AllocateDirect.java:
##########
@@ -111,15 +99,15 @@ StepBoolean getValid() {
}
@Override
- public void run() throws MemoryCloseException {
+ public void run() throws IllegalStateException {
deallocate(true);
}
- boolean deallocate(final boolean calledFromCleaner) throws MemoryCloseException {
+ boolean deallocate(final boolean calledFromCleaner) throws IllegalStateException {
if (valid.change()) {
if (calledFromCleaner) {
// Warn about non-deterministic resource cleanup.
- LOG.warning("A WritableHandle was not closed manually");
+ //LOG.warning("A direct resource was not closed explicitly");
Review Comment:
Why would this log no longer be useful to the user?
##########
datasketches-memory-java8/src/main/java/org/apache/datasketches/memory/internal/AllocateDirectWritableMap.java:
##########
@@ -291,15 +274,15 @@ StepBoolean getValid() {
}
@Override
- public void run() throws MemoryCloseException {
+ public void run() throws IllegalStateException {
deallocate(true);
}
- boolean deallocate(final boolean calledFromCleaner) throws MemoryCloseException {
+ boolean deallocate(final boolean calledFromCleaner) throws IllegalStateException {
if (valid.change()) {
if (calledFromCleaner) {
// Warn about non-deterministic resource cleanup.
- LOG.warning("A WritableMapHandleImpl was not closed manually");
+ //LOG.warning("A WritableMapHandleImpl was not closed manually");
Review Comment:
The same question applies here - do the exceptions now convey the same information that the log message warns of?
--
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: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-memory] leerho merged pull request #174: Add javadoc throws
Posted by "leerho (via GitHub)" <gi...@apache.org>.
leerho merged PR #174:
URL: https://github.com/apache/datasketches-memory/pull/174
--
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: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org