[GitHub] [lucene] ChrisHegarty commented on issue #12396: Make ForUtil Vectorized
ChrisHegarty commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615848352 If I'm not mistaken, the specific interface required here to abstract out `ForUtil` would look something like this? ``` interface ForUtilXXX { /** Number of bytes required to encode 128 integers of {@code bitsPerValue} bits per value. */ int numBytes(int bitsPerValue); /** Encodes integers from {@code values} into {@code out}. */ void encode(int[] values, int bitsPerValue, DataOutput out) throws IOException; /** Decodes integers into {@code values}. */ void decode(int bitsPerValue, DataInput in, int[] values) throws IOException; } ``` This interface does not need to be shared outside of its package - both the default and Panama implementations can reside in the same package. Maybe we need this interface for different codec versions? If so, then it could be copied, otherwise we need to find a "sharable" package. We current have a `TestSecrets`, that supports cross-package access, but even a more general non-test specific `o.a.l.internal.SharedSecrets` is not sufficient here, since we need to share both the Vector lookup code, and (maybe?) the interface, ForUtilXXX. The most straightforward, but less defensive, way I see is to double down on `org.apache.lucene.internal` as location to house such things that are Lucene internal **not to be used by external developers**. While not a very novel approach, it is well understood, commonly used, and adds the least amount of friction to the code. -- 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
[GitHub] [lucene] uschindler commented on issue #12396: Make ForUtil Vectorized
uschindler commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615851386 Hey, please for not let me do the integration. I have a plan already because with the current version it wont work. We have no secrets at moment, so the implementation can only be inside the utils package. if there are different versions per codec we would only implement the latest version with vectorization. The older codecs won't get vectorized. -- 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
[GitHub] [lucene] uschindler commented on issue #12396: Make ForUtil Vectorized
uschindler commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615852602 One we have some code here ready, I will submit a PR with the refactoring as preparation. -- 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
[GitHub] [lucene] stefanvodita opened a new pull request, #12409: Move sliced int buffer functionality to MemoryIndex
stefanvodita opened a new pull request, #12409: URL: https://github.com/apache/lucene/pull/12409 `SliceReader`, `SliceWriter`, and all other methods and fields having to do with slices in `IntBlockPool` are moved to `MemoryIndex.SlicedIntBlockPool`, as that is the only consumer of the slices functionality. There are a few other changes: 1. `IntBlockPool` is no longer `final`, which allows `SlicedIntBlockPool` to extend it. 2. I noticed a buffer pool test I had [committed previously](https://github.com/apache/lucene/pull/12392) was flawed and it only tested overflows for `ByteBlockPool`. I added the same test and corresponding fix for `IntBlockPool` and corrected the assertions. 3. All the existing `IntBlockPool` tests were testing slices. I moved those to their own test class and added a basic write/read/reset test in their place. -- 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
[GitHub] [lucene] tang-hi commented on issue #12396: Make ForUtil Vectorized
tang-hi commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615878090 > One we have some code here ready, I will submit a PR with the refactoring as preparation. I've had some things to take care of in the past few days 😅, so I haven't made much progress. I'll be providing specific implementation in the next few days. -- 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
[GitHub] [lucene] ChrisHegarty commented on issue #12396: Make ForUtil Vectorized
ChrisHegarty commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615888789 @uschindler Ok, thanks. Sounds like you have this under control. And what you say makes sense. Did the small sketch of the ForUtil interface I provided above make sense? Is that the level where we’re thinking that this will be abstracted? This is key to understanding how much flexibility we have in the underlying implementations. -- 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
[GitHub] [lucene] uschindler commented on issue #12396: Make ForUtil Vectorized
uschindler commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615890843 I like the idea to put everything under internal, we have that apckage already. But I would keep the implementations all under one package and let codes use them (specific impl per codec is ok, just different class names/getters in the provider interface). Let me provide the general setup later this weekend or begin of week. -- 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
[GitHub] [lucene] ChrisHegarty commented on issue #12396: Make ForUtil Vectorized
ChrisHegarty commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615897183 Thanks @uschindler, no great urgency on this part. As you have said earlier, work on the actual vector implementation can proceed independently of the general infrastructure - we can just copy the bits that we need for now, then refactor them out later. -- 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
[GitHub] [lucene] ChrisHegarty commented on issue #12396: Make ForUtil Vectorized
ChrisHegarty commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615898202 @tang-hi thanks for the update. I have some time later this weekend, I’ll see if I can get this started, and take a look at your code. -- 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
[GitHub] [lucene] uschindler commented on issue #12396: Make ForUtil Vectorized
uschindler commented on issue #12396: URL: https://github.com/apache/lucene/issues/12396#issuecomment-1615899286 I had some ideas in mind that the general lookup part just makes sure theres is a paname impl available and then delegates to the panama provider to decide which implementation to use. I plan to open an issue on JDK for 22 to have some way to detect which features are avail (like FMA). So we can also let the panama provider decide based on (future) features to return implementations based (like a full FMA impl of our current vector util) based on what the JDK reports back. We should really start and work on asking the Panama OpenJDK community to have some information about what features are available. The current state of not knowing anything is far from useful. -- 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