Re: [PR] Rewrite of the GroupVInt optimization without lambdas, varhandles and no code in subclasses [lucene]

[via GitHub]

uschindler commented on PR #15116:
URL: https://github.com/apache/lucene/pull/15116#issuecomment-3227184413

   > I see it on the overview page here: 
https://benchmarks.mikemccandless.com/2025.08.25.18.04.16.html, maybe you were 
looking at the wrong day?
   > 
   > <img alt="Capture d’écran 2025-08-27 à 09 36 29" width="669" height="95" 
src="https://private-user-images.githubusercontent.com/299848/482537259-d98037fd-1267-4393-9c02-05792694cf48.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NTYyODA5ODAsIm5iZiI6MTc1NjI4MDY4MCwicGF0aCI6Ii8yOTk4NDgvNDgyNTM3MjU5LWQ5ODAzN2ZkLTEyNjctNDM5My05YzAyLTA1NzkyNjk0Y2Y0OC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwODI3JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDgyN1QwNzQ0NDBaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT04OTgwNDJmMjczYjg3MmUwZjAxZTBhODU5YmM5ZTUwYTcwNjQ5NGM4MmU3NWMwNzc0ZDdkOWE1YjVjNTZmOGIyJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.6Mn_f5xtQXiVMxm_IoUUFaeBpgfkq0c945GFXpop9jg";>
   
   Yes I was on same page. The stupid thing is that you don't see the 
percentage change to previous run, you have to go through all numbers one by 
one to see the impact.
   
   I just checked the green numbers and all showed 0. Can anybody explain what 
they mean? What is the X? Why can't it show with more decial digits, so we see 
a factor like 1.035?
   
   > > So in short: let's keep this and backport it?
   > 
   > This is fine with me.
   
   Will do. I will still keep a bit of backwards compatibility so people don't 
fail when they implement the method in IndexInput. It is just not used.
   
   > > In followup I will try to reimplement ByteBuffersDirectory for main 
branch with a new MemorySegment variant based on current mmap code. But that's 
unrelated, it will only improve the NRTDirectory.
   > 
   > FWIW I also liked your other idea of merging IndexInput and 
RandomAccessInput together.
   
   Yeah, that's something long overdue. The RandomAccessIndexInput was only 
introduced at Lucene 3.x times when backwards compatibility was a high 
priority. At that time adding a method without a useful default implementation 
(which was not possible due to the seeking issue needing a separate instance) 
was not "allowed". So we had no other chance than adding a new interface that 
can be optionally implemented. We also added a getter to retrieve a "view" of 
the IndexInput as "random". Most "good" implementations just return itsself, 
but the default backwards compatibility layer returned a new clone instance 
which does seek/read pairs (and therefor modifies state). BTW, NIOFS and all 
BufferedIndexInputs have exactly the problem of requiring a clone at moment.
   
   I will use a step-by-step process:
   - First let IndexInput implement RandomAccessInput from beginning, so any 
implementing code fails to compile
   - Let the RandomAccess getter method return `this`
   - Then add impl for all failing subclasses
   - Then remove the getter and adapt code everywhere to fix types
   - Finally remove the interface
   
   I will need some time to do this, but 
   Uwe


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org