[SAMZA-966] Performance: Remove finalizer from RocksDbIterator

• Type: Bug
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Labels: newbie++
• Assignee: Unassigned
• Reporter: Jake Maes
• Created: 10/Jun/16 00:52
• Updated: 10/Jun/16 06:33

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