foundationdb icon indicating copy to clipboard operation
foundationdb copied to clipboard

Adding Finalizer timing to FDBTransaction.java

Open scottfines opened this issue 3 years ago • 5 comments

This adds a simple latency time measurement to calls made to FDBTransaction.java#finalize() so that the time spent in finalization can be measured.

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • [ ] The PR has a description, explaining both the problem and the solution.
  • [ ] The description mentions which forms of testing were done and the testing seems reasonable.
  • [ ] Every function/class/actor that was touched is reasonably well documented.

scottfines avatar Jan 22 '22 12:01 scottfines

Doxense CI Report for Windows 10

  • Commit ID: ea666ef580ca07ec056f08bd5f45199ec85fed4f
  • Result: SUCCEEDED
  • Build Logs (available dor 30 days)

fdb-windows-ci avatar Jan 22 '22 12:01 fdb-windows-ci

AWS CodeBuild CI Report for macOS Catalina 10.15

  • CodeBuild project: foundationdb-pr-macos
  • Commit ID: ea666ef580ca07ec056f08bd5f45199ec85fed4f
  • Result: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jan 22 '22 13:01 foundationdb-ci

AWS CodeBuild CI Report for Linux CentOS 7

  • CodeBuild project: foundationdb-pr
  • Commit ID: ea666ef580ca07ec056f08bd5f45199ec85fed4f
  • Result: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

foundationdb-ci avatar Jan 22 '22 13:01 foundationdb-ci

The finalizers are deprecated, and I'm hoping that we can remove them before too long. I don't have any concerns with adding this change, but just in case it changes anything for you I wanted to make you aware of that context.

sfc-gh-abeamon avatar Jan 23 '22 21:01 sfc-gh-abeamon

I don't think that's a particular problem from my perspective--if the finalizers are never called, then I don't have to worry about the latency within their call stack :)

scottfines avatar Jan 24 '22 11:01 scottfines

There is a PR removing the deprecated APIs, so this should no longer be needed.

jzhou77 avatar Oct 03 '22 16:10 jzhou77