You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "szetszwo (via GitHub)" <gi...@apache.org> on 2023/09/15 20:38:24 UTC

[GitHub] [ozone] szetszwo opened a new pull request, #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

szetszwo opened a new pull request, #5302:
URL: https://github.com/apache/ozone/pull/5302

   ## What changes were proposed in this pull request?
   
   CodecBuffer overrides the finalize() method for leak detection. It turns out causing a much longer GC time. (Thanks
   @duongkame for finding it out.)
   
   This JIRA is to change CodecBuffer not to override finalize(). In the test, we will create a subclass to override CodecBuffer for leak detection.
   
   ## What is the link to the Apache JIRA
   
   HDDS-9295
   
   ## How was this patch tested?
   
   Added a new test.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] kerneltime merged pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime merged PR #5302:
URL: https://github.com/apache/ozone/pull/5302


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #5302:
URL: https://github.com/apache/ozone/pull/5302#issuecomment-1728207928

   > ... A finalizer alternative, like reference queue, can be considered (ref: https://www.oracle.com/technical-resources/articles/javase/finalization.html).
   
   ReferenceQueue sounds like a very good idea.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] duongkame commented on a diff in pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5302:
URL: https://github.com/apache/ozone/pull/5302#discussion_r1330788915


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java:
##########
@@ -153,8 +188,11 @@ private void assertRefCnt(int expected) {
     Preconditions.assertSame(expected, buf.refCnt(), "refCnt");
   }
 
-  @Override
-  protected void finalize() throws Throwable {
+  /**
+   * Provide an implementation of the {@link #finalize()} method.
+   * For performance reason, this class does not override {@link #finalize()}.
+   */
+  void finalizeImpl() {

Review Comment:
   /nit can be named `detectLeaks`, no need to expose the details of finalize here. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #5302:
URL: https://github.com/apache/ozone/pull/5302#issuecomment-1726365336

   @umamaheswararao , we do have the background information in the first two comments of HDDS-9295.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] umamaheswararao commented on pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "umamaheswararao (via GitHub)" <gi...@apache.org>.
umamaheswararao commented on PR #5302:
URL: https://github.com/apache/ozone/pull/5302#issuecomment-1723907407

   @szetszwo @duongkame Could you please update the JIRA description about the background of this issue?


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on a diff in pull request #5302: HDDS-9295. Avoid overriding finalize() in CodecBuffer.

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5302:
URL: https://github.com/apache/ozone/pull/5302#discussion_r1332000778


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java:
##########
@@ -153,8 +188,11 @@ private void assertRefCnt(int expected) {
     Preconditions.assertSame(expected, buf.refCnt(), "refCnt");
   }
 
-  @Override
-  protected void finalize() throws Throwable {
+  /**
+   * Provide an implementation of the {@link #finalize()} method.
+   * For performance reason, this class does not override {@link #finalize()}.
+   */
+  void finalizeImpl() {

Review Comment:
   Done.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org