Lucene - Core
  1. Lucene - Core
  2. LUCENE-5700

Add 'accountable' interface for various ramBytesUsed

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.9, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Currently this is a disaster. there is ramBytesUsed(), sizeInBytes(), etc etc everywhere, with zero consistency, little javadocs, and no structure. For example, look at LUCENE-5695, where we go back and forth on how to handle "don't know".

      I don't think we should add any more of these methods to any classes in lucene until this has been cleaned up.

      1. LUCENE-5700.patch
        76 kB
        Adrien Grand

        Issue Links

          Activity

          Hide
          Michael McCandless added a comment -

          +1 to clean this up.

          Show
          Michael McCandless added a comment - +1 to clean this up.
          Hide
          Adrien Grand added a comment -

          Here is a patch:

          • new oal.util.Accountable interface with a ramBytesUsed() method
          • Classes that had a ramBytesUsed/sizeInBytes method to estimate memory usage now implement this interface
          • Classes that had a sizeInBytes method to compute disk usage remained as-is.

          I think the tough question is what to do in case memory usage cannot be computed. Returning -1 would work but we would need to make sure all consumers of this API handle that case properly... Since we don't have this issue now (all classes that implement the interface know how to do it), the documentation specifies that negative values are unsupported. Maybe we'll need to revisit it in the future in case the problem arises but for now I think that is the simplest option?

          Show
          Adrien Grand added a comment - Here is a patch: new oal.util.Accountable interface with a ramBytesUsed() method Classes that had a ramBytesUsed/sizeInBytes method to estimate memory usage now implement this interface Classes that had a sizeInBytes method to compute disk usage remained as-is. I think the tough question is what to do in case memory usage cannot be computed. Returning -1 would work but we would need to make sure all consumers of this API handle that case properly... Since we don't have this issue now (all classes that implement the interface know how to do it), the documentation specifies that negative values are unsupported. Maybe we'll need to revisit it in the future in case the problem arises but for now I think that is the simplest option?
          Hide
          Robert Muir added a comment -

          Since we don't have this issue now (all classes that implement the interface know how to do it)

          Yeah, I don't think a class should implement the interface if it can't actually return a valid result.

          Show
          Robert Muir added a comment - Since we don't have this issue now (all classes that implement the interface know how to do it) Yeah, I don't think a class should implement the interface if it can't actually return a valid result.
          Hide
          Dawid Weiss added a comment -

          I'm for throwing an exception. Either a class knows how to handle it or shouldn't implement it (throw UnsupportedOperationException).

          Show
          Dawid Weiss added a comment - I'm for throwing an exception. Either a class knows how to handle it or shouldn't implement it (throw UnsupportedOperationException).
          Hide
          ASF subversion and git services added a comment -

          Commit 1598470 from Adrien Grand in branch 'dev/trunk'
          [ https://svn.apache.org/r1598470 ]

          LUCENE-5700: Add oal.util.Accountable and make all classes that can compute their memory usage implement it.

          Show
          ASF subversion and git services added a comment - Commit 1598470 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1598470 ] LUCENE-5700 : Add oal.util.Accountable and make all classes that can compute their memory usage implement it.
          Hide
          Adrien Grand added a comment -

          Committed. Thanks Robert and Dawid for the feedback!

          Show
          Adrien Grand added a comment - Committed. Thanks Robert and Dawid for the feedback!
          Hide
          ASF subversion and git services added a comment -

          Commit 1598479 from Adrien Grand in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1598479 ]

          LUCENE-5700: Add oal.util.Accountable and make all classes that can compute their memory usage implement it.

          Show
          ASF subversion and git services added a comment - Commit 1598479 from Adrien Grand in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1598479 ] LUCENE-5700 : Add oal.util.Accountable and make all classes that can compute their memory usage implement it.

            People

            • Assignee:
              Adrien Grand
              Reporter:
              Robert Muir
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development