[GitHub] [lucene] uschindler commented on pull request #11982: Change ByteBuffersDataInput and ByteBuffersIndexInput to use absolute addressing

[GitBox <gi...@apache.org>]

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

   Hi, I can only look at this on the weekend as I am at a customer this week. When reading the description I was also on the wrong path anyways, because I did not understand what you want to change, because the getLongs() and getFloats() calls on IndexInput are always relative. I was not aware that you were talking about those crazy set of different view buffers for long/float that currently use the position() call to copy the position from main buffer.
   
   MY humble OPINION: I never agreed to that code and it is/was heavily broken (my personal opinion). Whenever I see that code I quickly look at other places just to not have the requirement to see it for more time (I get some "I need to puke!" reaction everytime I see it). In short: I would not spend too much time into byte buffers, sorry. MemorySegment is the way to go. In MemorySegmentIndexInput reading is a one liner and not views are needed, because MemorySegments allow unaligned accesses. In Java 19 you can also convert a ByteBuffer to a MemorySegment so you need no views anymore - I was thinking about at least fixing those 2 methods in ByteBuffer*s*IndexInput.
   
   About your benchmark, I do not trust it at the moment, because your code shows the follwoing warning on startup: "WARNING: Using incubator modules: jdk.incubator.vector". I have the feeling theres something in your code that uses this incubation module and may affect results. Where does the message comes from, I grepped through your patches, luckily you do not use incubator modules!
   
   When reading your attached log, where do you see an 10-20% improvement on IntNRQ or SSDV Facets?
                             IntNRQ      776.60     (21.0%)      795.66     (21.1%)    2.5% ( -32% -   56%) 0.712
   That says only 2.5%.
   
   In short, I have to closer look at the code, but I do not see much imrpovement, sorry. The numbers are now +/- idetical to MemorySegmentIndexInput for the given candidate queries.


-- 
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-unsubscribe@lucene.apache.org

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


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