[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-17 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1014529950 Here is the report of [making vint final](https://github.com/apache/lucene/compare/main...gf2121:make_vint_final?expand=1) (50 repeat * 20 JVM): ```

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-17 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1014390791 Thanks @uschindler ! > I was also thinking about the following: Why not make readVInt/readVLong final in the DataInput class? This should be possible after the refactoring in

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-16 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1014233876 To ensure the benchmark is warmed up, I also run a benchmark that repeat `AndHighLow` task 2 times (which was usually seen a good speed up in benchmarks before). Here is the re

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-16 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1014179236 As it can be the same as super, I tried to reuse super logic ([call super.xxx](https://github.com/apache/lucene/pull/592/commits/480f383fa74229389cc2acd44fa2e86ae0c71130)). still s

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-16 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1013928623 To test if it is the `try-catch` block preventing inline, I made another experiment: call `readByte()` instead of `guard.get(curBuf)` to read byte, like this (Yes it is just a copy

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-13 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1011957978 Thanks @jpountz . I opened https://issues.apache.org/jira/browse/LUCENE-10376 (https://github.com/apache/lucene/pull/602) It can be seen that `IntNRQ` is getting a 10% speed

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-12 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1011782903 Thanks all! I've finished benchmarks (20 JVM * 200 repeat): **Read byte from guard** > Here is the [code](https://github.com/apache/lucene/pull/592/commits/5fae0b2f4ffc6de

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-11 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1010612581 @uschindler Thank you for the benchmark guidance! I made a new change in this [commit](https://github.com/apache/lucene/pull/592/commits/347fdb13d60f620064290123f5122e6ea78ac

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-11 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1009907675 Thanks @jpountz @uschindler , this is the benchmark result based on the newest result ``` TaskQPS baseline StdDevQPS my_modified_version

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-10 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1009656951 I checked some other usage, seeing that for operations like `bytebuffer#readBytes` we also just checked once. If the `length` <= 6, DirectByteBuffer will read byte one by one, then

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-10 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1009639460 Thanks @jpountz ! I did not realize that reading ummapped bytebuffer will make JVM crach, sorry! > we should keep calling ensureValid before reading any byte from the ByteBu

[GitHub] [lucene] gf2121 commented on pull request #592: LUCENE-10366: Reduce the number of valid checks for ByteBufferIndexInput#readVInt

2022-01-10 Thread GitBox
gf2121 commented on pull request #592: URL: https://github.com/apache/lucene/pull/592#issuecomment-1009198243 Thanks @jpountz ! I did not realize that reading ummapped bytebuffer will make JVM crach, sorry! > Separately, this makes me wonder if some of the tasks that get the best sp