Details

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

      Description

      LUCENE-5463 tried to remove calls to RamUsageEstimator.sizeOf(Object) yet it was not always possible to remove the call when there was no other API to compute the memory usage of a particular class. In particular, this is the case for CachingWrapperFilter.sizeInBytes() that needs to be able to get the memory usage of any cacheable DocIdSet instance.

      We could add DocIdSet.ramBytesUsed in order to remove the need for RamUsageEstimator. This will also help have bounded filter caches and take the size of the cached doc id sets into account when doing evictions.

      1. LUCENE-5695.patch
        25 kB
        Adrien Grand
      2. LUCENE-5695.patch
        24 kB
        Adrien Grand
      3. LUCENE-5695.patch
        8 kB
        Adrien Grand

        Issue Links

          Activity

          Hide
          Adrien Grand added a comment -

          Here is a patch.

          Show
          Adrien Grand added a comment - Here is a patch.
          Hide
          Adrien Grand added a comment -

          I'm not too happy with the default implementation of ramBytesUsed (return 0) so we might want to keep it this way for 4.x but make it abstract in 5.0?

          Show
          Adrien Grand added a comment - I'm not too happy with the default implementation of ramBytesUsed ( return 0 ) so we might want to keep it this way for 4.x but make it abstract in 5.0?
          Hide
          Robert Muir added a comment -

          I think if we want to add a default impl, it should return -1 or throw UOE

          Otherwise there is no differentiation between "not implemented" and "doesnt use space"

          Show
          Robert Muir added a comment - I think if we want to add a default impl, it should return -1 or throw UOE Otherwise there is no differentiation between "not implemented" and "doesnt use space"
          Hide
          Adrien Grand added a comment -

          Here is a new patch that makes ramBytesUsed abstract for 5.x. For 4.x I'm thinking about making it throw an UOE.

          Show
          Adrien Grand added a comment - Here is a new patch that makes ramBytesUsed abstract for 5.x. For 4.x I'm thinking about making it throw an UOE.
          Hide
          Adrien Grand added a comment -

          I tried to make this method only exposed on doc id sets that can be cached by introducing a new CacheableDocIdSet that would implement Accountable while DocIdSet would not, but this doesn't play nicely with filtering (FilteredDocIdSet)...

          The attached patch uses the same approach as the previous one except that it makes DocIdSet implement Accountable instead of having its own ramBytesUsed method.

          Show
          Adrien Grand added a comment - I tried to make this method only exposed on doc id sets that can be cached by introducing a new CacheableDocIdSet that would implement Accountable while DocIdSet would not, but this doesn't play nicely with filtering (FilteredDocIdSet)... The attached patch uses the same approach as the previous one except that it makes DocIdSet implement Accountable instead of having its own ramBytesUsed method.
          Hide
          ASF subversion and git services added a comment -

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

          LUCENE-5695: DocIdSet implements Accountable.

          Show
          ASF subversion and git services added a comment - Commit 1602387 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1602387 ] LUCENE-5695 : DocIdSet implements Accountable.
          Hide
          ASF subversion and git services added a comment -

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

          LUCENE-5695: DocIdSet implements Accountable.

          Show
          ASF subversion and git services added a comment - Commit 1602391 from Adrien Grand in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1602391 ] LUCENE-5695 : DocIdSet implements Accountable.
          Hide
          ASF subversion and git services added a comment -

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

          LUCENE-5695: Fix 4.x merge: default implementation should be to throw an exception!

          Looks like somehow my fingers disagreed with my head when merging.

          Show
          ASF subversion and git services added a comment - Commit 1602540 from Adrien Grand in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1602540 ] LUCENE-5695 : Fix 4.x merge: default implementation should be to throw an exception! Looks like somehow my fingers disagreed with my head when merging.

            People

            • Assignee:
              Adrien Grand
              Reporter:
              Adrien Grand
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development