Skip to content

Merge main22 source set into main in simdvec#145005

Open
ChrisHegarty wants to merge 8 commits intoelastic:mainfrom
ChrisHegarty:merge-main22-into-main
Open

Merge main22 source set into main in simdvec#145005
ChrisHegarty wants to merge 8 commits intoelastic:mainfrom
ChrisHegarty:merge-main22-into-main

Conversation

@ChrisHegarty
Copy link
Contributor

This commit merges the main22 source set into main in the simdvec module, eliminating the last MR-JAR source set split. This is task 3 of #144797.

The 6 files in main22 were either stubs in main that returned Optional.empty() (ByteVectorScorer, FloatVectorScorer, Int7SQVectorScorer, Int7uOSQVectorScorer), a main22-only functional interface (SparseScorer), or a behavioral override adding native-access support (MemorySegmentES92Int7VectorsScorer). I replaced the stubs with the real implementations, moved SparseScorer into main, and merged the native-access branching into MemorySegmentES92Int7VectorsScorer.

To ensure these scorers are not invoked on JDK 21, where heap-backed MemorySegments cannot be passed to native downcalls, I added a SUPPORTS_HEAP_SEGMENTS guard at the top of each scorer's create() method, and conditioned MemorySegmentES92Int7VectorsScorer.NATIVE_SUPPORTED on it as well. This preserves the same runtime behavior as the old MR-JAR stubs.

While here, I also consolidated the SUPPORTS_HEAP_SEGMENTS constant, which was duplicated across 8+ files, into a single canonical definition in PanamaVectorConstants.

With no mainNN source sets remaining, I removed the jarHell disable hack from build.gradle. The elasticsearch.mrjar plugin is retained because it provides --enable-preview compilation and preview-bit stripping for the class files. We'll see what we can do about this later.

Note: With the MR-JAR split gone, the boundary between JDK 21 and JDK 22+ code paths is now less visible, it's now guarded by runtime checks rather than enforced by separate source sets. This means that it is easier to accidentally introduce a JDK 22+ assumption in a code path that runs on JDK 21. Until JDK 21 support is dropped, we should be careful when modifying scorer or MemorySegment-related code, and always verify with -Druntime.java=21. I do wonder if we could somehow add this to the CI! ?

@ChrisHegarty ChrisHegarty added >refactoring :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Mar 26, 2026
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
As for your note: I think this is already the case - I myself have run into that problem already. Regular CI does not catch it, but periodic does, and will open a GH issue if needed. But I wonder if we can add a test-jdk21 label or something to make this happen in the PR CI?

@ldematte
Copy link
Contributor

ldematte commented Mar 26, 2026

The elasticsearch.mrjar plugin is retained because it provides --enable-preview compilation and preview-bit stripping for the class files. We'll see what we can do about this later.

Definitely a follow-up, but maybe we can have a elasticsearch.preview plugin that does the same as mrjar, limited to --enable-preview compilation and preview-bit stripping for the class files.

Or we can even think to add that logic to the base plugin, and have it for everything?

@ChrisHegarty
Copy link
Contributor Author

LGTM As for your note: I think this is already the case - I myself have run into that problem already. Regular CI does not catch it, but periodic does, and will open a GH issue if needed. But I wonder if we can add a test-jdk21 label or something to make this happen in the PR CI?

Already in progress: #145007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>refactoring :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants