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

Feng Guo edited comment on LUCENE-9629 at 12/3/20, 6:43 AM:
------------------------------------------------------------

[~jpountz] Thanks for your reply! I can't agree more that write path is less 
performance-sensitive than the read path, and to be honest, i didn't expect 
this change will bring a very big improvement in writing speed. All I'm trying 
to do is just to reduce duplicate compute no matter where it appears. So you 
may think of it as a "fix" instead of an "enhancement".

here is a simple benchmark run with cpu profiler
{code:java}
public static void main(String[] args) throws Exception {
    Random random = new Random(System.currentTimeMillis());
    long[] nums = new long[128];
    for (int i = 0; i < 128; i++) {
        nums[i] = random.nextInt(7) + 1;
    }
    ForUtil forUtil = new ForUtil();
    DataOutput dataOutput = new DataOutput() {
        @Override
        public void writeLong(long i) throws IOException {}
        @Override
        public void writeByte(byte b) throws IOException {}
        @Override
        public void writeBytes(byte[] bytes, int i, int i1) throws IOException 
{}
    };
    while (true){
        forUtil.encode(nums, 3, dataOutput);
    }
}{code}
*result:*
|| method||before||after||
|org.apache.lucene.store.ForUtil.collapse8|29.9%|31.7%|
|org.apache.lucene.store.ForUtil.mask8|13.7%|2.8%|
|java.lang.Long.reverseBytes|< 1%|< 1%|
|org.apache.lucene.codecs.lucene84.Main$1.writeLong |< 1%|< 1%|

>From my point of view, the number of code lines is less important than writing 
>speed, and ForUtil is somewhat a hot way when indexing, so it may be worth 
>fixing. But if you insist that the precompute make no sense, just tell me and 
>i will revert this part of change.

In addition, i'm a bit poor in english speaking and most of words above come 
from translate programs. if there are any word offending you, please just 
ignore it. i really admire this amazing project and just try my best to make it 
better:)

 


was (Author: gf2121):
[~jpountz] Thanks for your reply! I can't agree more that write path is less 
performance-sensitive than the read path, and to be honest, i didn't expect 
this change will bring a very big improvement in writing speed. All I'm trying 
to do is just to reduce duplicate compute no matter where it appears. So you 
may think of it as a "fix" instead of an "enhancement".

here is a simple benchmark run with cpu profiler
{code:java}
public static void main(String[] args) throws Exception {
    Random random = new Random(System.currentTimeMillis());
    long[] nums = new long[128];
    for (int i = 0; i < 128; i++) {
        nums[i] = random.nextInt(7) + 1;
    }
    ForUtil forUtil = new ForUtil();
    DataOutput dataOutput = new DataOutput() {
        @Override
        public void writeLong(long i) throws IOException {}
        @Override
        public void writeByte(byte b) throws IOException {}
        @Override
        public void writeBytes(byte[] bytes, int i, int i1) throws IOException 
{}
    };
    while (true){
        forUtil.encode(nums, 3, dataOutput);
    }
}{code}
*result:*
|| ||before||after||
|org.apache.lucene.store.ForUtil.collapse8
 org.apache.lucene.store.ForUtil.mask8
java.lang.Long.reverseBytes
org.apache.lucene.codecs.lucene84.Main$1.writeLong|29.9%
 13.7%
< 1%
< 1%|31.7%
2.8%
< 1%
< 1%|

>From my point of view, the number of code lines is less important than writing 
>speed, and ForUtil is somewhat a hot way when indexing, so it may be worth 
>fixing. But if you insist that the precompute make no sense, just tell me and 
>i will revert this part of change.

In addition, i'm a bit poor in english speaking and most of words above come 
from translate programs. if there are any word offending you, please just 
ignore it. i really admire this amazing project and just try my best to make it 
better:)

 

> Use computed mask values in ForUtil
> -----------------------------------
>
>                 Key: LUCENE-9629
>                 URL: https://issues.apache.org/jira/browse/LUCENE-9629
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: core/codecs
>            Reporter: Feng Guo
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> In the class ForkUtil, mask values have been computed and stored in static 
> final vailables, but they are recomputed for every encoding, which may be 
> unnecessary. 
> anther small fix is that change
> {code:java}
> remainingBitsPerValue > remainingBitsPerLong{code}
>  to
> {code:java}
> remainingBitsPerValue >= remainingBitsPerLong{code}
> otherwise
> {code:java}
> if (remainingBitsPerValue == 0) {
>  idx++;
>  remainingBitsPerValue = bitsPerValue; 
> }
> {code}
> these code will never be used.



--
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