[ 
https://issues.apache.org/jira/browse/LUCENE-10112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17417580#comment-17417580
 ] 

Uwe Schindler edited comment on LUCENE-10112 at 9/20/21, 11:09 AM:
-------------------------------------------------------------------

bq. TestBackwardsCompatibility does not address my concerns about using 
ByteOrder.nativeOrder()

Robert, calm down and maybe drink a coffee first! You just misunderstood me! In 
my first sentence I said that I agree.

My concern was just the following: In Lucene 8.x we have all LZ4 encoding with 
hashes based on BE integers (look at the "old" code). Our requirement is: An 
index saved on disk and created with those previous version must be readable in 
9.0, regardless of the endianness we use. And thats asserted by the 
TestBackwardsCompatibility: It checks that LE-based hashes in 8.x are still 
readable and decode correctly.

Again in very short: Test BackwardsCompatibility ensures that an index written 
with 8.x is readable and stored fields decompress correctly, although in 8.x we 
used BE and now we use hardcoded LE (as YOU suggested). This had nothing to do 
with nativeOrder().


was (Author: thetaphi):
bq. TestBackwardsCompatibility does not address my concerns about using 
ByteOrder.nativeOrder()

Robert, calm down and maybe drink a coffee first! You just misunderstood me! In 
my first sentence I said that I agree.

My concern was just the following: In Lucene 8.x we have all LZ4 encoding with 
hashes based on BE integers (look at the "old" code). Our requirement is: An 
index saved on disk and created with those previous version must be readable in 
9.0, regardless of the endianness we use. And thats asserted by the 
TestBackwardsCompatibility: It checks that LE-based hashes in 8.x are still 
readable and decode correctly.

Again in very short: Test BackwardsCompatibility ensures that an index written 
with 8.x is readable and stored fields decompress correctly, although in 8.x we 
used BE and now we use hardcode LE (as YOU suggested). This had nothing to do 
with nativeOrder().

> Improve LZ4 Compression performance with direct primitive read/writes
> ---------------------------------------------------------------------
>
>                 Key: LUCENE-10112
>                 URL: https://issues.apache.org/jira/browse/LUCENE-10112
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: Tim Brooks
>            Assignee: Uwe Schindler
>            Priority: Minor
>         Attachments: LUCENE-10112.patch
>
>
> *Summary*
> Java9 introduced VarHandles as a tool to quickly read and write primitive 
> types directly to byte arrays without bound checks. The LZ4 compressor class 
> must consistently read ints from a byte array to analyze matches. The 
> performance can be improved by reading these using a VarHandle.
> Additionally, the LZ4 compressor/decompressor methods currently individually 
> read/write the bytes for LE shorts. Lucene's DataOutput/DataInput 
> abstractions already have dedicated methods for reading/writing LE shorts. 
> These methods are selectively optimized in certain implementations and will 
> provide superior performance than individual byte reads.
> *Concerns*
> The DataOutput/DataInput readShort() and writeShort() methods do not call out 
> that they are LE. It just looks to me that the DataOutput/DataInput are LE? 
> Since this particular change does not appear to provide significant 
> performance wins, maybe the patch is better leaving the explicit individual 
> byte reads?
> Additionally, this patch changes read ints to read them in the platform 
> native order which should be fine since it is just matching bytes. But I can 
> change it to only read in the order the previous version did.
> *Benchmarks*
> I created JMH benchmarks which compresses 1MB of highly compressible JSON 
> observability data. And compresses it 64KB at a time. In order to simulate 
> the "short" changes, I use a forked version `ByteArrayDataOutput` which 
> writes shorts using a VarHandle (to simulate fast writes that the ByteBuffer 
> versions would get.) I also ran a benchmark without the short changes, just 
> the reading ints using a VarHandle.
>  
>  
> {noformat}
> Benchmark                                          Mode  Cnt    Score   Error 
>  Units
> MyBenchmark.testCompressLuceneLZ4                 thrpt    9  712.430 ± 3.616 
>  ops/s
> MyBenchmark.testCompressLuceneLZ4Forked           thrpt    9  945.380 ± 4.776 
>  ops/s
> MyBenchmark.testCompressLuceneLZ4ForkedNoShort    thrpt    9  940.812 ± 3.868 
>  ops/s
> MyBenchmark.testCompressLuceneLZ4HC               thrpt    9  147.432 ± 4.730 
>  ops/s
> MyBenchmark.testCompressLuceneLZ4HCForked         thrpt    9  183.954 ± 2.534 
>  ops/s
> MyBenchmark.testCompressLuceneLZ4HCForkedNoShort  thrpt    9  188.065 ± 0.727 
>  ops/s{noformat}
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to