Uploaded image for project: 'Samza'
  1. Samza
  2. SAMZA-966

Performance: Remove finalizer from RocksDbIterator

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      It is well-known that finalizable objects hurt GC performance. Josh Bloch covers it in Effective Java.

      We have one finalizer in Samza for RocksDbIterator. This is bad because:
      1. RocksDbIterators can be created and destroyed very frequently and making them disposable makes it harder for the nursery to clean them up.
      2. It's not necessary. Finalize calls close() which calls dispose() on RocksObject. The javaDoc for that method is pretty clear:

      /**

      • Release the c++ object manually pointed by the native handle.
      • <p>
      • Note that
        Unknown macro: {@code dispose()}

        will also be called during the GC process

        • if it was not called before its
          Unknown macro: {@code RocksObject}

          went out-of-scope.

        • However, since Java may wrongly wrongly assume those objects are
        • small in that they seems to only hold a long variable. As a result,
        • they might have low priority in the GC process. To prevent this,
        • it is suggested to call

        manually.

      • </p>
      • <p>
      • Note that once an instance of
        Unknown macro: {@code RocksObject}

        has been disposed,

      • calling its function will lead undefined behavior.
      • </p>
        */

      Sounds like the memory will eventually be freed. So we're paying a tax just to play big brother.

      Instead, I think we should document that KeyValueIterator should be always be closed, remove the finalizer from RocksDbIterator, and update KeyValueIterator to also extend Closeable, which will enable the try-with-resources pattern
      https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              jmakes Jake Maes
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated: