Lucene - Core
  1. Lucene - Core
  2. LUCENE-1574

PooledSegmentReader, pools SegmentReader underlying byte arrays

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.4.1
    • Fix Version/s: 4.9, 5.0
    • Component/s: modules/other
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      PooledSegmentReader pools the underlying byte arrays of deleted docs and norms for realtime search. It is designed for use with IndexReader.clone which can create many copies of byte arrays, which are of the same length for a given segment. When pooled they can be reused which could save on memory.

      Do we want to benchmark the memory usage comparison of PooledSegmentReader vs GC? Many times GC is enough for these smaller objects.

      1. LUCENE-1574.patch
        26 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        Presumably it wouldn't save on memory (the pool would presumably sometimes be holding onto spares, for future reuse), but could save on time, right?

        Or, maybe instead we could spend our effort making a simple transactional data structure for holding deletes/norms (I think there's already an issue on this – maybe it's LUCENE-1526).

        Show
        Michael McCandless added a comment - Presumably it wouldn't save on memory (the pool would presumably sometimes be holding onto spares, for future reuse), but could save on time, right? Or, maybe instead we could spend our effort making a simple transactional data structure for holding deletes/norms (I think there's already an issue on this – maybe it's LUCENE-1526 ).
        Hide
        Jason Rutherglen added a comment -

        True the pool would hold onto spares, but they would expire.
        It's mostly useful for the large on disk segments as those byte
        arrays (for BitVectors) are large, and because there's more docs
        in them would get hit with deletes more often, and so they'd be
        reused fairly often.

        I'm not knowledgeable enough to say whether the transactional
        data structure will be fast enough. We had been using
        http://fastutil.dsi.unimi.it/docs/it/unimi/dsi/fastutil/ints/IntR
        BTreeSet.html in Zoie for deleted docs and it's way slow. Binary
        search of an int array is faster, albeit not fast enough. The
        multi dimensional array thing isn't fast enough (for searching)
        as we implemented this in Bobo. It's implemented in Bobo because
        we have a multi value field cache (which is quite large because
        for each doc we're storing potentially 64 or more values in an
        inplace bitset) and a single massive array kills the GC. In some
        cases this is faster than a single large array because of the
        way Java (or the OS?) transfers memory around through the CPU
        cache.

        Show
        Jason Rutherglen added a comment - True the pool would hold onto spares, but they would expire. It's mostly useful for the large on disk segments as those byte arrays (for BitVectors) are large, and because there's more docs in them would get hit with deletes more often, and so they'd be reused fairly often. I'm not knowledgeable enough to say whether the transactional data structure will be fast enough. We had been using http://fastutil.dsi.unimi.it/docs/it/unimi/dsi/fastutil/ints/IntR BTreeSet.html in Zoie for deleted docs and it's way slow. Binary search of an int array is faster, albeit not fast enough. The multi dimensional array thing isn't fast enough (for searching) as we implemented this in Bobo. It's implemented in Bobo because we have a multi value field cache (which is quite large because for each doc we're storing potentially 64 or more values in an inplace bitset) and a single massive array kills the GC. In some cases this is faster than a single large array because of the way Java (or the OS?) transfers memory around through the CPU cache.
        Hide
        Michael McCandless added a comment -

        Moving out.

        Show
        Michael McCandless added a comment - Moving out.
        Hide
        John Wang added a comment -

        Re: Zoie and deleted docs:
        That is no longer true, Zoie is using a bloom filter over a intHash set from fastutil for exactly the perf reason Jason pointed.

        Show
        John Wang added a comment - Re: Zoie and deleted docs: That is no longer true, Zoie is using a bloom filter over a intHash set from fastutil for exactly the perf reason Jason pointed.
        Hide
        Jason Rutherglen added a comment -

        Yonik,

        Do you recommend using the method in SimpleStringInterner for lockless pooling?

        Show
        Jason Rutherglen added a comment - Yonik, Do you recommend using the method in SimpleStringInterner for lockless pooling?
        Hide
        Jason Rutherglen added a comment -

        I suppose as we're on Java 1.5, ConcurrentLinkedQueue can be used.

        Show
        Jason Rutherglen added a comment - I suppose as we're on Java 1.5, ConcurrentLinkedQueue can be used.
        Hide
        Jason Rutherglen added a comment -

        A likely optimization for this patch is we'll only pool if the doc count is above a threshold, 100,000 seems like a good number. Also pooling will be optional.

        Show
        Jason Rutherglen added a comment - A likely optimization for this patch is we'll only pool if the doc count is above a threshold, 100,000 seems like a good number. Also pooling will be optional.
        Hide
        Jason Rutherglen added a comment -

        I'm going to revive this, and if it works fine for trunk, then we can use the basic system for RT eg, LUCENE-2312. I think the only open question is how we'll shrink the pool, most likely there'd be an expiration on the pooled objects. With RT, the parallel arrays will grow, so the pool will need to be size based, eg, when the arrays are grown, all of the previous arrays may be forcefully evicted, or they may simply expire.

        Show
        Jason Rutherglen added a comment - I'm going to revive this, and if it works fine for trunk, then we can use the basic system for RT eg, LUCENE-2312 . I think the only open question is how we'll shrink the pool, most likely there'd be an expiration on the pooled objects. With RT, the parallel arrays will grow, so the pool will need to be size based, eg, when the arrays are grown, all of the previous arrays may be forcefully evicted, or they may simply expire.
        Hide
        Michael McCandless added a comment -

        I'm working on an initial patch for this...

        I think the only open question is how we'll shrink the pool, most likely there'd be an expiration on the pooled objects.

        I think we can simply have a "max pooled free bit vectors"... or we may want to expire by time/staleness as well.

        With RT, the parallel arrays will grow, so the pool will need to be size based, eg, when the arrays are grown, all of the previous arrays may be forcefully evicted, or they may simply expire.

        True... but, like the other per-doc arrays, the BV can be overallocated (ArrayUtil.oversize) to accommodate further added docs.

        Show
        Michael McCandless added a comment - I'm working on an initial patch for this... I think the only open question is how we'll shrink the pool, most likely there'd be an expiration on the pooled objects. I think we can simply have a "max pooled free bit vectors"... or we may want to expire by time/staleness as well. With RT, the parallel arrays will grow, so the pool will need to be size based, eg, when the arrays are grown, all of the previous arrays may be forcefully evicted, or they may simply expire. True... but, like the other per-doc arrays, the BV can be overallocated (ArrayUtil.oversize) to accommodate further added docs.
        Hide
        Jason Rutherglen added a comment -

        ThreadPoolExecutor can act as a guide, it's main parameters are corePoolSize, maximumPoolSize, keepAliveTime.

        In regards to using System.arraycopy for the RT parallel arrays, if they grow to become too large, then SC could become a predominant cost. However if the default thread states is 8, which'd mean that many DWPTs, the arrays would probably never grow to be too large for their SC to become noticeably expensive, hopefully.

        Show
        Jason Rutherglen added a comment - ThreadPoolExecutor can act as a guide, it's main parameters are corePoolSize, maximumPoolSize, keepAliveTime. In regards to using System.arraycopy for the RT parallel arrays, if they grow to become too large, then SC could become a predominant cost. However if the default thread states is 8, which'd mean that many DWPTs, the arrays would probably never grow to be too large for their SC to become noticeably expensive, hopefully.
        Hide
        Jason Rutherglen added a comment -

        We want to record the deletes between getReader calls however there's no way to know ahead of time if a term or query is going to hit many documents or not, meaning we can't always save del docids, because we'd have too many ints in RAM. I'm curious how we plan on handling this case?

        Show
        Jason Rutherglen added a comment - We want to record the deletes between getReader calls however there's no way to know ahead of time if a term or query is going to hit many documents or not, meaning we can't always save del docids, because we'd have too many ints in RAM. I'm curious how we plan on handling this case?
        Hide
        Michael McCandless added a comment -

        I'm curious how we plan on handling this case?

        I think we should keep the replay log smallish, or, expire it aggressively with age. I suspect this opto is only going to be "worth it" for very frequent reopens... but I'm not sure yet.

        Show
        Michael McCandless added a comment - I'm curious how we plan on handling this case? I think we should keep the replay log smallish, or, expire it aggressively with age. I suspect this opto is only going to be "worth it" for very frequent reopens... but I'm not sure yet.
        Hide
        Michael McCandless added a comment -

        Attached rough patch.

        At least one test fails....

        And, I haven't yet seen that this is in fact worthwhile. The rough benchmark I have (which hits other issues so the results aren't conclusive yet) doesn't show much difference w/ this patch. I think this patch may only be worthwhile at insane reopen rates, which I think in practice is rarely a legitimate use case (even though many apps start off thinking it is).

        Show
        Michael McCandless added a comment - Attached rough patch. At least one test fails.... And, I haven't yet seen that this is in fact worthwhile. The rough benchmark I have (which hits other issues so the results aren't conclusive yet) doesn't show much difference w/ this patch. I think this patch may only be worthwhile at insane reopen rates, which I think in practice is rarely a legitimate use case (even though many apps start off thinking it is).
        Hide
        Jason Rutherglen added a comment -

        What size segments is the benchmark deleting against? Maybe we're underestimating the speed of arraycopy, eg, it's really a hardware operation that could be optimized?

        Show
        Jason Rutherglen added a comment - What size segments is the benchmark deleting against? Maybe we're underestimating the speed of arraycopy, eg, it's really a hardware operation that could be optimized?
        Hide
        Michael McCandless added a comment -

        I've been testing on a 25M doc index (all of en Wikipedia, at least as of March 2010).

        Yes, I think likely alloc of big BitVector, System.arraycopy, destroying it, may be a fairly low cost compared to lucene resolving the deleted term, indexing the doc, flushing the tiny segment, etc.

        Show
        Michael McCandless added a comment - I've been testing on a 25M doc index (all of en Wikipedia, at least as of March 2010). Yes, I think likely alloc of big BitVector, System.arraycopy, destroying it, may be a fairly low cost compared to lucene resolving the deleted term, indexing the doc, flushing the tiny segment, etc.
        Hide
        Jason Rutherglen added a comment -

        I think likely alloc of big BitVector, System.arraycopy, destroying it, may be a fairly low cost compared to lucene resolving the deleted term, indexing the doc, flushing the tiny segment, etc.

        Right, and if we pool the byte[]s we'd take the cost of instantiating and GC'ing out of the picture in the high NRT throughput case. It's counter intuitive and will require testing.

        Show
        Jason Rutherglen added a comment - I think likely alloc of big BitVector, System.arraycopy, destroying it, may be a fairly low cost compared to lucene resolving the deleted term, indexing the doc, flushing the tiny segment, etc. Right, and if we pool the byte[]s we'd take the cost of instantiating and GC'ing out of the picture in the high NRT throughput case. It's counter intuitive and will require testing.
        Hide
        Steve Rowe added a comment -

        Bulk move 4.4 issues to 4.5 and 5.0

        Show
        Steve Rowe added a comment - Bulk move 4.4 issues to 4.5 and 5.0
        Hide
        Uwe Schindler added a comment -

        Move issue to Lucene 4.9.

        Show
        Uwe Schindler added a comment - Move issue to Lucene 4.9.

          People

          • Assignee:
            Unassigned
            Reporter:
            Jason Rutherglen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Time Tracking

              Estimated:
              Original Estimate - 168h
              168h
              Remaining:
              Remaining Estimate - 168h
              168h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development