Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.1, 5.0
    • Component/s: modules/facet
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      Today, IntEncoder/Decoder offer a streaming API, where you can encode(int) and decode(int). Originally, we believed that this layer can be useful for other scenarios, but in practice it's used only for writing/reading the category ordinals from payload/DV.

      Therefore, Mike and I would like to explore a bulk API, something like encode(IntsRef, BytesRef) and decode(BytesRef, IntsRef). Perhaps the Encoder can still be streaming (as we don't know in advance how many ints will be written), dunno. Will figure this out as we go.

      One thing to check is whether the bulk API can work w/ e.g. facet associations, which can write arbitrary byte[], and so may decoding to an IntsRef won't make sense. This too we'll figure out as we go. I don't rule out that associations will use a different bulk API.

      At the end of the day, the requirement is for someone to be able to configure how ordinals are written (i.e. different encoding schemes: VInt, PackedInts etc.) and later read, with as little overhead as possible.

      1. LUCENE-4620.patch
        250 kB
        Shai Erera
      2. LUCENE-4620.patch
        239 kB
        Shai Erera
      3. LUCENE-4620.patch
        244 kB
        Shai Erera
      4. LUCENE-4620.patch
        1 kB
        Michael McCandless
      5. LUCENE-4620.patch
        3 kB
        Michael McCandless
      6. LUCENE-4620.patch
        37 kB
        Shai Erera

        Activity

        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Shai Erera
        http://svn.apache.org/viewvc?view=revision&revision=1432034

        LUCENE-4620: IntEncoder/Decoder bulk API

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Shai Erera http://svn.apache.org/viewvc?view=revision&revision=1432034 LUCENE-4620 : IntEncoder/Decoder bulk API
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Shai Erera
        http://svn.apache.org/viewvc?view=revision&revision=1433929

        LUCENE-4620: inline encoding/decoding

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Shai Erera http://svn.apache.org/viewvc?view=revision&revision=1433929 LUCENE-4620 : inline encoding/decoding
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Shai Erera
        http://svn.apache.org/viewvc?view=revision&revision=1433926

        LUCENE-4620: inline encoding/decoding

        Show
        Commit Tag Bot added a comment - [trunk commit] Shai Erera http://svn.apache.org/viewvc?view=revision&revision=1433926 LUCENE-4620 : inline encoding/decoding
        Hide
        Shai Erera added a comment -

        Could be DV helps some too. Also not calling decode() + reset() + doDecode() every time must help some too.

        Committed the changes to trunk, 4x and 4.1 branch.

        Show
        Shai Erera added a comment - Could be DV helps some too. Also not calling decode() + reset() + doDecode() every time must help some too. Committed the changes to trunk, 4x and 4.1 branch.
        Hide
        Michael McCandless added a comment -

        +1

        It's much faster than I had tested before (maybe because of the DV cutover!?):

                            Task    QPS base      StdDev    QPS comp      StdDev                Pct diff
                        PKLookup      181.98      (1.2%)      182.20      (1.3%)    0.1% (  -2% -    2%)
                         LowTerm       77.95      (2.0%)       83.59      (2.8%)    7.2% (   2% -   12%)
                         MedTerm       26.60      (3.3%)       31.46      (1.4%)   18.3% (  13% -   23%)
                        HighTerm       15.83      (3.9%)       19.35      (1.3%)   22.2% (  16% -   28%)
        
        Show
        Michael McCandless added a comment - +1 It's much faster than I had tested before (maybe because of the DV cutover!?): Task QPS base StdDev QPS comp StdDev Pct diff PKLookup 181.98 (1.2%) 182.20 (1.3%) 0.1% ( -2% - 2%) LowTerm 77.95 (2.0%) 83.59 (2.8%) 7.2% ( 2% - 12%) MedTerm 26.60 (3.3%) 31.46 (1.4%) 18.3% ( 13% - 23%) HighTerm 15.83 (3.9%) 19.35 (1.3%) 22.2% ( 16% - 28%)
        Hide
        Shai Erera added a comment -

        Attached patch:

        • Inlines VInt8 encode/decode in relevant encoders/decdoers.
        • Marks encoders/decoders final.
        • Gets rid of the decode() + doDecode(). It was nice while I wrote it, but I figure that this is a hot code, and every method call counts, especially when called for few values usually.
        • Decoders no longer mess w/ bytes.offset (now that the decoding is inlined).
        • Removed VInt8 class and test.

        Mike, would you like to run luceneutil with this patch?

        Show
        Shai Erera added a comment - Attached patch: Inlines VInt8 encode/decode in relevant encoders/decdoers. Marks encoders/decoders final. Gets rid of the decode() + doDecode(). It was nice while I wrote it, but I figure that this is a hot code, and every method call counts, especially when called for few values usually. Decoders no longer mess w/ bytes.offset (now that the decoding is inlined). Removed VInt8 class and test. Mike, would you like to run luceneutil with this patch?
        Hide
        Michael McCandless added a comment -

        I think we should just make a specialized accumulator/aggregator, for the counts-only-dgap-vint case: that could wouldn't need to populate an IntsRef and then make 2nd pass over the ords ... it'd just increment the count for each ord as it decodes in. In previous issues I already tested that this gives a good gain ...

        Show
        Michael McCandless added a comment - I think we should just make a specialized accumulator/aggregator, for the counts-only-dgap-vint case: that could wouldn't need to populate an IntsRef and then make 2nd pass over the ords ... it'd just increment the count for each ord as it decodes in. In previous issues I already tested that this gives a good gain ...
        Hide
        Michael McCandless added a comment -

        Patch, fixing that bug Shai found.

        Performance is better with this specialization:

                            Task    QPS base      StdDev    QPS comp      StdDev                Pct diff
                        PKLookup      192.61      (4.5%)      193.06      (4.2%)    0.2% (  -8% -    9%)
                         LowTerm       15.33      (1.6%)       15.44      (2.5%)    0.7% (  -3% -    4%)
                         MedTerm        7.60      (0.7%)        7.74      (1.8%)    1.9% (   0% -    4%)
                        HighTerm        3.85      (0.6%)        3.97      (1.2%)    3.1% (   1% -    4%)
        

        I also tried the unrolling of the vInt loop but perf was strangely quite a bit worse..

        Show
        Michael McCandless added a comment - Patch, fixing that bug Shai found. Performance is better with this specialization: Task QPS base StdDev QPS comp StdDev Pct diff PKLookup 192.61 (4.5%) 193.06 (4.2%) 0.2% ( -8% - 9%) LowTerm 15.33 (1.6%) 15.44 (2.5%) 0.7% ( -3% - 4%) MedTerm 7.60 (0.7%) 7.74 (1.8%) 1.9% ( 0% - 4%) HighTerm 3.85 (0.6%) 3.97 (1.2%) 3.1% ( 1% - 4%) I also tried the unrolling of the vInt loop but perf was strangely quite a bit worse..
        Hide
        Shai Erera added a comment - - edited

        I see. I have two comments about the patch. This part is wrong:

        +    int needed = upto - buf.offset;
        +    if (values.length < needed) {
        +      values.grow(needed);
        +    }
        

        should be

        +    if (values.ints.length < buf.length) {
        +      values.grow(buf.length);
        +    }
        

        With your patch, values.grow() is always called, even if inside it doesn't do anything.
        I wonder if we should not grow() the array, but rather grow it from the outside ourselves. Because IntsRef.grow() checks the capacity again (and Robert is against grow() anyway...).

        Also, note how this way you check offset < upto on every byte read while in the current code it's checked only once per integer read. Maybe if you do a while loop inside the loop, something like while (b < 0).

        Show
        Shai Erera added a comment - - edited I see. I have two comments about the patch. This part is wrong: + int needed = upto - buf.offset; + if (values.length < needed) { + values.grow(needed); + } should be + if (values.ints.length < buf.length) { + values.grow(buf.length); + } With your patch, values.grow() is always called, even if inside it doesn't do anything. I wonder if we should not grow() the array, but rather grow it from the outside ourselves. Because IntsRef.grow() checks the capacity again (and Robert is against grow() anyway...). Also, note how this way you check offset < upto on every byte read while in the current code it's checked only once per integer read. Maybe if you do a while loop inside the loop, something like while (b < 0) .
        Hide
        Michael McCandless added a comment -

        Maybe doing bulk-vInt-decode (see patch) will be faster (just make hotspot's job easier) ... I'll test.

        Show
        Michael McCandless added a comment - Maybe doing bulk-vInt-decode (see patch) will be faster (just make hotspot's job easier) ... I'll test.
        Hide
        Shai Erera added a comment -

        I made this change to VInt8IntDecoder instead of checking inside the loop:

        int numValues = buf.length; // a value occupies at least 1 byte
        if (values.ints.length < numValues) {
          values.grow(numValues);
        }
        

        Ran EncodingSpeed again and compared the results. On average (4 datasets), VInt8 achieves a 0.69% speedup, DGap(VInt) 7.85% and Sorting(Unique(DGap(VInt))) 10.16%. The last one is the default Encoder, thought its decoder is only DGap(VInt), so I'm not sure why the difference between that run and the previous one with 7.85%.

        However, it does look like it speeds things up...

        Show
        Shai Erera added a comment - I made this change to VInt8IntDecoder instead of checking inside the loop: int numValues = buf.length; // a value occupies at least 1 byte if (values.ints.length < numValues) { values.grow(numValues); } Ran EncodingSpeed again and compared the results. On average (4 datasets), VInt8 achieves a 0.69% speedup, DGap(VInt) 7.85% and Sorting(Unique(DGap(VInt))) 10.16%. The last one is the default Encoder, thought its decoder is only DGap(VInt), so I'm not sure why the difference between that run and the previous one with 7.85%. However, it does look like it speeds things up...
        Hide
        Shai Erera added a comment - - edited

        I'm baffled too. There is some overhead with the bulk API, in that it needs to grow() the IntsRef (something it didn't need to do before). But I believe that this growing should stabilize after few docs (i.e. the array becomes large enough). Still, every iteration checks if the array is large enough, so perhaps if we grow the IntsRef upfront (even if too much), we can remove the 'ifs'.

        SimpleIntDecoder can do it easily, it knows there are 4 bytes per value, so it should just grow by buf.length / 4. VInt is more tricky, but to be on the safe side it can grow by buf.length, as at the minimum each value occupies only one byte. Some other decoders are trickier, but they are not in effect in your test above.

        But I must admit that I thought it's a no brainer that replacing an iterator API by a bulk is going to improve performance. And indeed, EncodingSpeed shows nice improvements already. And even if decoding values is not the major part of faceted search (which I doubt), we shouldn't see slowdowns? At the most we shouldn't see big wins?

        Show
        Shai Erera added a comment - - edited I'm baffled too. There is some overhead with the bulk API, in that it needs to grow() the IntsRef (something it didn't need to do before). But I believe that this growing should stabilize after few docs (i.e. the array becomes large enough). Still, every iteration checks if the array is large enough, so perhaps if we grow the IntsRef upfront (even if too much), we can remove the 'ifs'. SimpleIntDecoder can do it easily, it knows there are 4 bytes per value, so it should just grow by buf.length / 4. VInt is more tricky, but to be on the safe side it can grow by buf.length, as at the minimum each value occupies only one byte. Some other decoders are trickier, but they are not in effect in your test above. But I must admit that I thought it's a no brainer that replacing an iterator API by a bulk is going to improve performance. And indeed, EncodingSpeed shows nice improvements already. And even if decoding values is not the major part of faceted search (which I doubt), we shouldn't see slowdowns? At the most we shouldn't see big wins?
        Hide
        Michael McCandless added a comment -

        This change seemed to lose a bit of performance: look at 1/11/2013 on http://people.apache.org/~mikemccand/lucenebench/TermDateFacets.html

        But, that tests just one dimension (Date), with only 3 ords per doc,
        so I had assumed that this just wasn't enough ints being decoded to
        see the gains from this bulk decoding.

        So, I modified luceneutil to have more facets per doc (avg ~25 ords
        per doc across 9 dimensions; 2.5M unique ords), and the results are
        still slower:

              Task    QPS base      StdDev    QPS comp      StdDev                Pct diff
          HighTerm        3.62      (2.5%)        3.24      (1.0%)  -10.5% ( -13% -   -7%)
           MedTerm        7.34      (1.7%)        6.78      (0.9%)   -7.6% ( -10% -   -5%)
           LowTerm       14.92      (1.6%)       14.32      (1.2%)   -4.0% (  -6% -   -1%)
          PKLookup      181.47      (4.7%)      183.04      (5.3%)    0.9% (  -8% -   11%)
        

        This is baffling ... not sure what's up. I would expect some gains
        given that the micro-benchmark showed sizable decode improvements. It
        must somehow be that decode cost is a minor part of facet counting?
        (which is not a good sign!: it should be a big part of it...)

        Show
        Michael McCandless added a comment - This change seemed to lose a bit of performance: look at 1/11/2013 on http://people.apache.org/~mikemccand/lucenebench/TermDateFacets.html But, that tests just one dimension (Date), with only 3 ords per doc, so I had assumed that this just wasn't enough ints being decoded to see the gains from this bulk decoding. So, I modified luceneutil to have more facets per doc (avg ~25 ords per doc across 9 dimensions; 2.5M unique ords), and the results are still slower: Task QPS base StdDev QPS comp StdDev Pct diff HighTerm 3.62 (2.5%) 3.24 (1.0%) -10.5% ( -13% - -7%) MedTerm 7.34 (1.7%) 6.78 (0.9%) -7.6% ( -10% - -5%) LowTerm 14.92 (1.6%) 14.32 (1.2%) -4.0% ( -6% - -1%) PKLookup 181.47 (4.7%) 183.04 (5.3%) 0.9% ( -8% - 11%) This is baffling ... not sure what's up. I would expect some gains given that the micro-benchmark showed sizable decode improvements. It must somehow be that decode cost is a minor part of facet counting? (which is not a good sign!: it should be a big part of it...)
        Hide
        Shai Erera added a comment -

        Committed to 4x and trunk.

        Show
        Shai Erera added a comment - Committed to 4x and trunk.
        Hide
        Shai Erera added a comment -

        I'll open an issue to take care of FacetFields reusability

        Done. Opened LUCENE-4680.

        Show
        Shai Erera added a comment - I'll open an issue to take care of FacetFields reusability Done. Opened LUCENE-4680 .
        Hide
        Shai Erera added a comment -

        I think we should remove it

        Ok I will.

        It is unfortunate that the common case is often held back by the full flexibility/generality of the facet module

        With LUCENE-4647, the common case suffers less from the full generality of the facets module. I'll open an issue to take care of FacetFields reusability and there I hope I'll be able to tackle successfully the reusability of BytesRefs for one as well as many CLPs.

        IMO though, having a single entry point for users to index facets, be it 1 facet per document, or 2500 (a real case!), is important. We need to make sure though that the 1 facet case is added the least overhead (e.g. using Collections.singletonMap, or the trick I've done in CountingListBuilder.OrdinalsEncoder (with/out partitions)).

        Show
        Shai Erera added a comment - I think we should remove it Ok I will. It is unfortunate that the common case is often held back by the full flexibility/generality of the facet module With LUCENE-4647 , the common case suffers less from the full generality of the facets module. I'll open an issue to take care of FacetFields reusability and there I hope I'll be able to tackle successfully the reusability of BytesRefs for one as well as many CLPs. IMO though, having a single entry point for users to index facets, be it 1 facet per document, or 2500 (a real case!), is important. We need to make sure though that the 1 facet case is added the least overhead (e.g. using Collections.singletonMap, or the trick I've done in CountingListBuilder.OrdinalsEncoder (with/out partitions)).
        Hide
        Michael McCandless added a comment -

        Trunk:

             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 3630 (unsorted, length of: 2430) 41152 times.
             [java] 
             [java] Encoder                Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                       [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                   18.4955                 4430                    44.3003                 1162                    11.6201
             [java] Sorting (Unique (VInt8))    18.4955                 4344                    43.4403                 1105                    11.0501
             [java] Sorting (Unique (DGap (VInt8)))     8.5597                 4481                    44.8103                  842                     8.4201
             [java] Sorting (Unique (DGap (EightFlags (VInt8))))     4.9679                 4636                    46.3603                 1021                    10.2101
             [java] Sorting (Unique (DGap (FourFlags (VInt8))))     4.8198                 4515                    45.1503                 1001                    10.0101
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8)))))     4.5794                 4904                    49.0403                 1056                    10.5601
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8)))))     4.5794                 4751                    47.5103                 1035                    10.3501
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 9910 (unsorted, length of: 1489) 67159 times.
             [java] 
             [java] Encoder                Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                       [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                   18.2673                 1241                    12.4100                 1128                    11.2800
             [java] Sorting (Unique (VInt8))    18.2673                 3488                    34.8801                  924                     9.2400
             [java] Sorting (Unique (DGap (VInt8)))     8.9456                 3061                    30.6101                  660                     6.6000
             [java] Sorting (Unique (DGap (EightFlags (VInt8))))     5.7542                 3693                    36.9301                 1026                    10.2600
             [java] Sorting (Unique (DGap (FourFlags (VInt8))))     5.5447                 3462                    34.6201                  811                     8.1100
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8)))))     5.3566                 3846                    38.4601                 1018                    10.1800
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8)))))     5.3996                 3879                    38.7901                 1025                    10.2500
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 10000 (unsorted, length of: 18) 5555555 times.
             [java] 
             [java] Encoder                Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                       [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                   20.8889                 1179                    11.7900                 1114                    11.1400
             [java] Sorting (Unique (VInt8))    20.8889                 2251                    22.5100                 1171                    11.7100
             [java] Sorting (Unique (DGap (VInt8)))    12.0000                 2174                    21.7400                  848                     8.4800
             [java] Sorting (Unique (DGap (EightFlags (VInt8))))    10.2222                 2372                    23.7200                 1092                    10.9200
             [java] Sorting (Unique (DGap (FourFlags (VInt8))))    10.2222                 2355                    23.5500                 1062                    10.6200
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8)))))     9.7778                 2414                    24.1400                 1085                    10.8500
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8)))))    10.2222                 2492                    24.9200                 1130                    11.3000
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 501871 (unsorted, length of: 957) 104493 times.
             [java] 
             [java] Encoder                Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                       [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                   16.5768                  998                     9.9800                  896                     8.9600
             [java] Sorting (Unique (VInt8))    16.5768                 2542                    25.4201                  864                     8.6400
             [java] Sorting (Unique (DGap (VInt8)))     8.4848                 2468                    24.6800                  646                     6.4600
             [java] Sorting (Unique (DGap (EightFlags (VInt8))))     4.4138                 2526                    25.2601                  768                     7.6800
             [java] Sorting (Unique (DGap (FourFlags (VInt8))))     4.1797                 2406                    24.0600                  696                     6.9600
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8)))))     3.8955                 2541                    25.4101                  802                     8.0200
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8)))))     3.8871                 2537                    25.3701                  770                     7.7000
             [java] 
        

        Patch:

             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 3630 (unsorted, length of: 2430) 41152 times.
             [java] 
             [java] Encoder                                                        Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                                                               [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                                                           18.4955                  594                     5.9400                  419                     4.1900
             [java] Sorting (Unique (VInt8))                                        18.4955                 3147                    31.4702                  579                     5.7900
             [java] Sorting (Unique (DGap (VInt8)))                                  8.5597                 3167                    31.6702                  278                     2.7800
             [java] Sorting (Unique (DGap (EightFlags (VInt))))                      4.9679                 3624                    36.2402                  401                     4.0100
             [java] Sorting (Unique (DGap (FourFlags (VInt))))                       4.8198                 3534                    35.3402                  379                     3.7900
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt)))))           4.5794                 3954                    39.5403                  580                     5.8000
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt)))))           4.5794                 3947                    39.4703                  595                     5.9500
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 9910 (unsorted, length of: 1489) 67159 times.
             [java] 
             [java] Encoder                                                        Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                                                               [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                                                           18.2673                  592                     5.9200                  441                     4.4100
             [java] Sorting (Unique (VInt8))                                        18.2673                 2002                    20.0200                  443                     4.4300
             [java] Sorting (Unique (DGap (VInt8)))                                  8.9456                 2077                    20.7701                  301                     3.0100
             [java] Sorting (Unique (DGap (EightFlags (VInt))))                      5.7542                 2646                    26.4601                  419                     4.1900
             [java] Sorting (Unique (DGap (FourFlags (VInt))))                       5.5447                 2505                    25.0501                  375                     3.7500
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt)))))           5.3566                 2984                    29.8401                  625                     6.2500
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt)))))           5.3996                 2997                    29.9701                  616                     6.1600
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 10000 (unsorted, length of: 18) 5555555 times.
             [java] 
             [java] Encoder                                                        Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                                                               [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                                                           20.8889                  585                     5.8500                  585                     5.8500
             [java] Sorting (Unique (VInt8))                                        20.8889                 1127                    11.2700                  588                     5.8800
             [java] Sorting (Unique (DGap (VInt8)))                                 12.0000                 1156                    11.5600                  477                     4.7700
             [java] Sorting (Unique (DGap (EightFlags (VInt))))                     10.2222                 1346                    13.4600                  657                     6.5700
             [java] Sorting (Unique (DGap (FourFlags (VInt))))                      10.2222                 1385                    13.8500                  573                     5.7300
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt)))))           9.7778                 1565                    15.6500                  845                     8.4500
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt)))))          10.2222                 1662                    16.6200                  891                     8.9100
             [java] 
             [java] 
             [java] Estimating ~100000000 Integers compression time by
             [java] Encoding/decoding facets' ID payload of docID = 501871 (unsorted, length of: 957) 104493 times.
             [java] 
             [java] Encoder                                                        Bits/Int          Encode Time                Encode Time          Decode Time                Decode Time
             [java]                                                                               [milliseconds]        [microsecond / int]       [milliseconds]        [microsecond / int]
             [java] -----------------------------------------------------------------------------------------------------------------------------------------------------------------------
             [java] VInt8                                                           16.5768                  446                     4.4600                  439                     4.3900
             [java] Sorting (Unique (VInt8))                                        16.5768                 1429                    14.2900                  420                     4.2000
             [java] Sorting (Unique (DGap (VInt8)))                                  8.4848                 1390                    13.9000                  298                     2.9800
             [java] Sorting (Unique (DGap (EightFlags (VInt))))                      4.4138                 1457                    14.5700                  387                     3.8700
             [java] Sorting (Unique (DGap (FourFlags (VInt))))                       4.1797                 1529                    15.2900                  368                     3.6800
             [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt)))))           3.8955                 1829                    18.2900                  530                     5.3000
             [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt)))))           3.8871                 1842                    18.4200                  528                     5.2800
             [java] 
        

        Looks like ~2-3X faster... good!

        Show
        Michael McCandless added a comment - Trunk: [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 3630 (unsorted, length of: 2430) 41152 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 18.4955 4430 44.3003 1162 11.6201 [java] Sorting (Unique (VInt8)) 18.4955 4344 43.4403 1105 11.0501 [java] Sorting (Unique (DGap (VInt8))) 8.5597 4481 44.8103 842 8.4201 [java] Sorting (Unique (DGap (EightFlags (VInt8)))) 4.9679 4636 46.3603 1021 10.2101 [java] Sorting (Unique (DGap (FourFlags (VInt8)))) 4.8198 4515 45.1503 1001 10.0101 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8))))) 4.5794 4904 49.0403 1056 10.5601 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8))))) 4.5794 4751 47.5103 1035 10.3501 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 9910 (unsorted, length of: 1489) 67159 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 18.2673 1241 12.4100 1128 11.2800 [java] Sorting (Unique (VInt8)) 18.2673 3488 34.8801 924 9.2400 [java] Sorting (Unique (DGap (VInt8))) 8.9456 3061 30.6101 660 6.6000 [java] Sorting (Unique (DGap (EightFlags (VInt8)))) 5.7542 3693 36.9301 1026 10.2600 [java] Sorting (Unique (DGap (FourFlags (VInt8)))) 5.5447 3462 34.6201 811 8.1100 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8))))) 5.3566 3846 38.4601 1018 10.1800 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8))))) 5.3996 3879 38.7901 1025 10.2500 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 10000 (unsorted, length of: 18) 5555555 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 20.8889 1179 11.7900 1114 11.1400 [java] Sorting (Unique (VInt8)) 20.8889 2251 22.5100 1171 11.7100 [java] Sorting (Unique (DGap (VInt8))) 12.0000 2174 21.7400 848 8.4800 [java] Sorting (Unique (DGap (EightFlags (VInt8)))) 10.2222 2372 23.7200 1092 10.9200 [java] Sorting (Unique (DGap (FourFlags (VInt8)))) 10.2222 2355 23.5500 1062 10.6200 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8))))) 9.7778 2414 24.1400 1085 10.8500 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8))))) 10.2222 2492 24.9200 1130 11.3000 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 501871 (unsorted, length of: 957) 104493 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 16.5768 998 9.9800 896 8.9600 [java] Sorting (Unique (VInt8)) 16.5768 2542 25.4201 864 8.6400 [java] Sorting (Unique (DGap (VInt8))) 8.4848 2468 24.6800 646 6.4600 [java] Sorting (Unique (DGap (EightFlags (VInt8)))) 4.4138 2526 25.2601 768 7.6800 [java] Sorting (Unique (DGap (FourFlags (VInt8)))) 4.1797 2406 24.0600 696 6.9600 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt8))))) 3.8955 2541 25.4101 802 8.0200 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt8))))) 3.8871 2537 25.3701 770 7.7000 [java] Patch: [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 3630 (unsorted, length of: 2430) 41152 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 18.4955 594 5.9400 419 4.1900 [java] Sorting (Unique (VInt8)) 18.4955 3147 31.4702 579 5.7900 [java] Sorting (Unique (DGap (VInt8))) 8.5597 3167 31.6702 278 2.7800 [java] Sorting (Unique (DGap (EightFlags (VInt)))) 4.9679 3624 36.2402 401 4.0100 [java] Sorting (Unique (DGap (FourFlags (VInt)))) 4.8198 3534 35.3402 379 3.7900 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt))))) 4.5794 3954 39.5403 580 5.8000 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt))))) 4.5794 3947 39.4703 595 5.9500 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 9910 (unsorted, length of: 1489) 67159 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 18.2673 592 5.9200 441 4.4100 [java] Sorting (Unique (VInt8)) 18.2673 2002 20.0200 443 4.4300 [java] Sorting (Unique (DGap (VInt8))) 8.9456 2077 20.7701 301 3.0100 [java] Sorting (Unique (DGap (EightFlags (VInt)))) 5.7542 2646 26.4601 419 4.1900 [java] Sorting (Unique (DGap (FourFlags (VInt)))) 5.5447 2505 25.0501 375 3.7500 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt))))) 5.3566 2984 29.8401 625 6.2500 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt))))) 5.3996 2997 29.9701 616 6.1600 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 10000 (unsorted, length of: 18) 5555555 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 20.8889 585 5.8500 585 5.8500 [java] Sorting (Unique (VInt8)) 20.8889 1127 11.2700 588 5.8800 [java] Sorting (Unique (DGap (VInt8))) 12.0000 1156 11.5600 477 4.7700 [java] Sorting (Unique (DGap (EightFlags (VInt)))) 10.2222 1346 13.4600 657 6.5700 [java] Sorting (Unique (DGap (FourFlags (VInt)))) 10.2222 1385 13.8500 573 5.7300 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt))))) 9.7778 1565 15.6500 845 8.4500 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt))))) 10.2222 1662 16.6200 891 8.9100 [java] [java] [java] Estimating ~100000000 Integers compression time by [java] Encoding/decoding facets' ID payload of docID = 501871 (unsorted, length of: 957) 104493 times. [java] [java] Encoder Bits/Int Encode Time Encode Time Decode Time Decode Time [java] [milliseconds] [microsecond / int] [milliseconds] [microsecond / int] [java] ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- [java] VInt8 16.5768 446 4.4600 439 4.3900 [java] Sorting (Unique (VInt8)) 16.5768 1429 14.2900 420 4.2000 [java] Sorting (Unique (DGap (VInt8))) 8.4848 1390 13.9000 298 2.9800 [java] Sorting (Unique (DGap (EightFlags (VInt)))) 4.4138 1457 14.5700 387 3.8700 [java] Sorting (Unique (DGap (FourFlags (VInt)))) 4.1797 1529 15.2900 368 3.6800 [java] Sorting (Unique (DGap (NOnes (3) (FourFlags (VInt))))) 3.8955 1829 18.2900 530 5.3000 [java] Sorting (Unique (DGap (NOnes (4) (FourFlags (VInt))))) 3.8871 1842 18.4200 528 5.2800 [java] Looks like ~2-3X faster... good!
        Hide
        Michael McCandless added a comment -

        Can we use Collections.singletonMap when there are no partitions?

        Done. Note though that BytesRef cannot be reused in the case of PerDimensionIndexingParams (i.e. multiple CLPs). This is not the common case, but it's not trivial to specialize it. Maybe as a second iteration. I did put a TODO in FacetFields to allow reuse.

        Well, we'd somehow need N BytesRefs to reuse (one per CLP) ... but I
        don't think we should worry about that now.

        It is unfortunate that the common case is often held back by the full
        flexibility/generality of the facet module ... sometimes I think we
        need a facet-light module. But maybe if we can get the specialization
        done we don't need facet-light ...

        why do we have VInt8.bytesNeeded? Who uses that?

        Currently no one uses it, but it was there and I thought that it's a convenient API to keep. Why encode and then see how many bytes were occupied?
        Anyway, neither the encoders nor the decoders use it. I have no strong feelings for keeping/removing it, so if you feel like it should be removed, I can do it.

        I think we should remove it: it's a dangerous API because it can
        encourage consumers to do things like call bytesNeeded first (to know
        how much to grow their buffer, say) followed by encoding. The slow
        part of vInt encoding is all those ifs ...

        Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef

        It is, but that's the result of Java's lack of pass by reference. I.e., decode needs to return the caller two values: the decoded number and how many bytes were read.
        Notice that in the previous byte[] variant, the method took a class Position, which is horrible. That's why I documented in decode() that it advances bytes.offset, so
        the caller can restore it in the end. For instance, IntDecoder restores the offset to the original one in the end.

        On LUCENE-4675 Robert gave me an idea to create a BytesRefIterator, and I started to play with it. I.e. it would wrap a BytesRef but add 'pos' and 'upto' indexes.
        The user can modify 'pos' freely, withouth touching bytes.offset. That introduces an object allocation though, and since I'd want to reuse that object wherever
        possible, I think I'll look at it after finishing this issue. It already contains too many changes.

        OK.

        I guess this is why you want an upto

        No, I wanted upto because iterating up to bytes.length is incorrect. You need to iterate up to offset+length. BytesRefIterator.pos and BytesRefIterator.upto solve these cases for me.

        OK.

        looks like things got a bit slower (or possibly it's noise)

        First, even if it's not noise, the slowdown IMO is worth the code simplification.

        +1

        But, I do believe that we'll see gains when there are more than 3 integers to encode/decode.
        In fact, the facets test package has an EncodingSpeed class which measures the time it takes to encode/decode a large number of integers (a few thousands). When I compared the
        result to 4x (i.e. without the patch), the decode time seemed to be ~x5 faster.

        Good! Would be nice to have a real-world biggish-number-of-facets
        benchmark ... I'll ponder how to do that w/ luceneutil.

        In this patch I added an Ant task "run-encoding-benchmark" which runs this class. Want to give it a try on your beast machine? For 4x, you can just copy the target to lucene/facet/build.xml, I believe it will work without issues.

        OK I'll run it!

        Show
        Michael McCandless added a comment - Can we use Collections.singletonMap when there are no partitions? Done. Note though that BytesRef cannot be reused in the case of PerDimensionIndexingParams (i.e. multiple CLPs). This is not the common case, but it's not trivial to specialize it. Maybe as a second iteration. I did put a TODO in FacetFields to allow reuse. Well, we'd somehow need N BytesRefs to reuse (one per CLP) ... but I don't think we should worry about that now. It is unfortunate that the common case is often held back by the full flexibility/generality of the facet module ... sometimes I think we need a facet-light module. But maybe if we can get the specialization done we don't need facet-light ... why do we have VInt8.bytesNeeded? Who uses that? Currently no one uses it, but it was there and I thought that it's a convenient API to keep. Why encode and then see how many bytes were occupied? Anyway, neither the encoders nor the decoders use it. I have no strong feelings for keeping/removing it, so if you feel like it should be removed, I can do it. I think we should remove it: it's a dangerous API because it can encourage consumers to do things like call bytesNeeded first (to know how much to grow their buffer, say) followed by encoding. The slow part of vInt encoding is all those ifs ... Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef It is, but that's the result of Java's lack of pass by reference. I.e., decode needs to return the caller two values: the decoded number and how many bytes were read. Notice that in the previous byte[] variant, the method took a class Position, which is horrible. That's why I documented in decode() that it advances bytes.offset, so the caller can restore it in the end. For instance, IntDecoder restores the offset to the original one in the end. On LUCENE-4675 Robert gave me an idea to create a BytesRefIterator, and I started to play with it. I.e. it would wrap a BytesRef but add 'pos' and 'upto' indexes. The user can modify 'pos' freely, withouth touching bytes.offset. That introduces an object allocation though, and since I'd want to reuse that object wherever possible, I think I'll look at it after finishing this issue. It already contains too many changes. OK. I guess this is why you want an upto No, I wanted upto because iterating up to bytes.length is incorrect. You need to iterate up to offset+length. BytesRefIterator.pos and BytesRefIterator.upto solve these cases for me. OK. looks like things got a bit slower (or possibly it's noise) First, even if it's not noise, the slowdown IMO is worth the code simplification. +1 But, I do believe that we'll see gains when there are more than 3 integers to encode/decode. In fact, the facets test package has an EncodingSpeed class which measures the time it takes to encode/decode a large number of integers (a few thousands). When I compared the result to 4x (i.e. without the patch), the decode time seemed to be ~x5 faster. Good! Would be nice to have a real-world biggish-number-of-facets benchmark ... I'll ponder how to do that w/ luceneutil. In this patch I added an Ant task "run-encoding-benchmark" which runs this class. Want to give it a try on your beast machine? For 4x, you can just copy the target to lucene/facet/build.xml, I believe it will work without issues. OK I'll run it!
        Hide
        Shai Erera added a comment -

        Can we use Collections.singletonMap when there are no partitions?

        Done. Note though that BytesRef cannot be reused in the case of PerDimensionIndexingParams (i.e. multiple CLPs). This is not the common case, but it's not trivial to specialize it. Maybe as a second iteration. I did put a TODO in FacetFields to allow reuse.

        why do we have VInt8.bytesNeeded? Who uses that?

        Currently no one uses it, but it was there and I thought that it's a convenient API to keep. Why encode and then see how many bytes were occupied?
        Anyway, neither the encoders nor the decoders use it. I have no strong feelings for keeping/removing it, so if you feel like it should be removed, I can do it.

        Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef

        It is, but that's the result of Java's lack of pass by reference. I.e., decode needs to return the caller two values: the decoded number and how many bytes were read.
        Notice that in the previous byte[] variant, the method took a class Position, which is horrible. That's why I documented in decode() that it advances bytes.offset, so
        the caller can restore it in the end. For instance, IntDecoder restores the offset to the original one in the end.

        On LUCENE-4675 Robert gave me an idea to create a BytesRefIterator, and I started to play with it. I.e. it would wrap a BytesRef but add 'pos' and 'upto' indexes.
        The user can modify 'pos' freely, withouth touching bytes.offset. That introduces an object allocation though, and since I'd want to reuse that object wherever
        possible, I think I'll look at it after finishing this issue. It already contains too many changes.

        I guess this is why you want an upto

        No, I wanted upto because iterating up to bytes.length is incorrect. You need to iterate up to offset+length. BytesRefIterator.pos and BytesRefIterator.upto solve these cases for me.

        looks like things got a bit slower (or possibly it's noise)

        First, even if it's not noise, the slowdown IMO is worth the code simplification. But, I do believe that we'll see gains when there are more than 3 integers to encode/decode.
        In fact, the facets test package has an EncodingSpeed class which measures the time it takes to encode/decode a large number of integers (a few thousands). When I compared the
        result to 4x (i.e. without the patch), the decode time seemed to be ~x5 faster.

        In this patch I added an Ant task "run-encoding-benchmark" which runs this class. Want to give it a try on your beast machine? For 4x, you can just copy the target to lucene/facet/build.xml, I believe it will work without issues.

        Show
        Shai Erera added a comment - Can we use Collections.singletonMap when there are no partitions? Done. Note though that BytesRef cannot be reused in the case of PerDimensionIndexingParams (i.e. multiple CLPs). This is not the common case, but it's not trivial to specialize it. Maybe as a second iteration. I did put a TODO in FacetFields to allow reuse. why do we have VInt8.bytesNeeded? Who uses that? Currently no one uses it, but it was there and I thought that it's a convenient API to keep. Why encode and then see how many bytes were occupied? Anyway, neither the encoders nor the decoders use it. I have no strong feelings for keeping/removing it, so if you feel like it should be removed, I can do it. Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef It is, but that's the result of Java's lack of pass by reference. I.e., decode needs to return the caller two values: the decoded number and how many bytes were read. Notice that in the previous byte[] variant, the method took a class Position, which is horrible. That's why I documented in decode() that it advances bytes.offset, so the caller can restore it in the end. For instance, IntDecoder restores the offset to the original one in the end. On LUCENE-4675 Robert gave me an idea to create a BytesRefIterator, and I started to play with it. I.e. it would wrap a BytesRef but add 'pos' and 'upto' indexes. The user can modify 'pos' freely, withouth touching bytes.offset. That introduces an object allocation though, and since I'd want to reuse that object wherever possible, I think I'll look at it after finishing this issue. It already contains too many changes. I guess this is why you want an upto No, I wanted upto because iterating up to bytes.length is incorrect. You need to iterate up to offset+length. BytesRefIterator.pos and BytesRefIterator.upto solve these cases for me. looks like things got a bit slower (or possibly it's noise) First, even if it's not noise, the slowdown IMO is worth the code simplification. But, I do believe that we'll see gains when there are more than 3 integers to encode/decode. In fact, the facets test package has an EncodingSpeed class which measures the time it takes to encode/decode a large number of integers (a few thousands). When I compared the result to 4x (i.e. without the patch), the decode time seemed to be ~x5 faster. In this patch I added an Ant task "run-encoding-benchmark" which runs this class. Want to give it a try on your beast machine? For 4x, you can just copy the target to lucene/facet/build.xml, I believe it will work without issues.
        Hide
        Michael McCandless added a comment -

        Thanks Shai, that new patch worked!

        This patch looks great!

        It's a little disturbing that every doc must make a new
        HashMap<String,BytesRef> at indexing time (seems like a lot of
        overhead/objects when the common case just needs to return a single
        BytesRef, which could be re-used). Can we use
        Collections.singletonMap when there are no partitions?

        The decode API (more important than encode) looks like it reuses the
        Bytes/IntsRef, so that's good.

        Hmm why do we have VInt8.bytesNeeded? Who uses that? I think that's
        a dangerous API to have .... it's better to simply encode and then see
        how many bytes it took.

        Hmm, it's a little abusive how VInt8.decode changes the offset of the
        incoming BytesRef ... I guess this is why you want an upto

        Net/net this is great progress over what we have today, so +1!

        I ran a quick 10M English Wikipedia test w/ just term queries:

        Task    QPS base      StdDev    QPS comp      StdDev                Pct diff
                       HighTerm       12.79      (2.4%)       12.56      (1.2%)   -1.8% (  -5% -    1%)
                        MedTerm       18.04      (1.8%)       17.77      (0.8%)   -1.5% (  -4% -    1%)
                        LowTerm       47.69      (1.1%)       47.56      (1.0%)   -0.3% (  -2% -    1%)
        

        The test only has 3 ords per doc so it's not "typical" ... looks like things got a bit slower (or possibly it's noise).

        Show
        Michael McCandless added a comment - Thanks Shai, that new patch worked! This patch looks great! It's a little disturbing that every doc must make a new HashMap<String,BytesRef> at indexing time (seems like a lot of overhead/objects when the common case just needs to return a single BytesRef, which could be re-used). Can we use Collections.singletonMap when there are no partitions? The decode API (more important than encode) looks like it reuses the Bytes/IntsRef, so that's good. Hmm why do we have VInt8.bytesNeeded? Who uses that? I think that's a dangerous API to have .... it's better to simply encode and then see how many bytes it took. Hmm, it's a little abusive how VInt8.decode changes the offset of the incoming BytesRef ... I guess this is why you want an upto Net/net this is great progress over what we have today, so +1! I ran a quick 10M English Wikipedia test w/ just term queries: Task QPS base StdDev QPS comp StdDev Pct diff HighTerm 12.79 (2.4%) 12.56 (1.2%) -1.8% ( -5% - 1%) MedTerm 18.04 (1.8%) 17.77 (0.8%) -1.5% ( -4% - 1%) LowTerm 47.69 (1.1%) 47.56 (1.0%) -0.3% ( -2% - 1%) The test only has 3 ords per doc so it's not "typical" ... looks like things got a bit slower (or possibly it's noise).
        Hide
        Shai Erera added a comment -

        Sorry. Can you try now?

        Show
        Shai Erera added a comment - Sorry. Can you try now?
        Hide
        Michael McCandless added a comment -

        Looks like there were some svn mv's, so the patch doesn't directly apply ...

        Can you regenerate the patch using 'svn diff --show-copies-as-adds' (assuming you're using svn 1.7+)?

        Either that or use dev-tools/scripts/diffSources.py ... thanks.

        Show
        Michael McCandless added a comment - Looks like there were some svn mv's, so the patch doesn't directly apply ... Can you regenerate the patch using 'svn diff --show-copies-as-adds' (assuming you're using svn 1.7+)? Either that or use dev-tools/scripts/diffSources.py ... thanks.
        Hide
        Shai Erera added a comment -

        Patch makes the following changes:

        • IntEncoder.encode() takes an IntsRef and BytesRef and encodes the integers from IntsRef to BytesRef. Similarily, IntDecoder.decode() takes a BytesRef and IntsRef and decodes the integers from the byte array to the integer array.
        • CategoryListIterator and Aggregator were changed to do bulk handling of category ordinals as well.
        • In the process I merged some methods such as PayloadIterator.setdoc and PayloadIterator.getPayload, as well as AssociationsPayloadIterator, to reduce even further the number of method calls that happen during search.
        • Added a test which tests MultiCategoryListIterator (we didn't have one!) and improved EncodingTest to test a large number of random values.

        All tests pass, and 'ant javadocs' passes too.

        Show
        Shai Erera added a comment - Patch makes the following changes: IntEncoder.encode() takes an IntsRef and BytesRef and encodes the integers from IntsRef to BytesRef . Similarily, IntDecoder.decode() takes a BytesRef and IntsRef and decodes the integers from the byte array to the integer array. CategoryListIterator and Aggregator were changed to do bulk handling of category ordinals as well. In the process I merged some methods such as PayloadIterator.setdoc and PayloadIterator.getPayload , as well as AssociationsPayloadIterator , to reduce even further the number of method calls that happen during search. Added a test which tests MultiCategoryListIterator (we didn't have one!) and improved EncodingTest to test a large number of random values. All tests pass, and 'ant javadocs' passes too.
        Hide
        Shai Erera added a comment -

        Also, today there are few IntEncoders which are used during indexing only, e.g. SortingIntEncoder and UniqueIntEncoder which guarantee that an ordinal will be written just once to the payload, and sort them so that DGap can be computed afterwards. These do not have a matching Decoder, and they shouldn't have, because at search time you don't care if the ords are sorted or not, and you can assume they are unique.

        Another thing that I think we should do is move those encoders into the *.facet package. They are currently under the facet module, but o.a.l.util, b/c again we thought at the time that they are a generic piece of code for encoding/decoding integers. Lucene has PackedInts and DataInput/Output for doing block and VInt encodings. Users can write Codecs for other encoding algorithms ... IntEncoder/Decoder are not that generic .

        Show
        Shai Erera added a comment - Also, today there are few IntEncoders which are used during indexing only, e.g. SortingIntEncoder and UniqueIntEncoder which guarantee that an ordinal will be written just once to the payload, and sort them so that DGap can be computed afterwards. These do not have a matching Decoder, and they shouldn't have, because at search time you don't care if the ords are sorted or not, and you can assume they are unique. Another thing that I think we should do is move those encoders into the *.facet package. They are currently under the facet module, but o.a.l.util, b/c again we thought at the time that they are a generic piece of code for encoding/decoding integers. Lucene has PackedInts and DataInput/Output for doing block and VInt encodings. Users can write Codecs for other encoding algorithms ... IntEncoder/Decoder are not that generic .

          People

          • Assignee:
            Shai Erera
            Reporter:
            Shai Erera
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development