Skip to content

Make some native vectorization classes package private to be consistent and a minor fix#15866

Merged
shubhamvishu merged 3 commits into
apache:mainfrom
shubhamvishu:native-private
Mar 24, 2026
Merged

Make some native vectorization classes package private to be consistent and a minor fix#15866
shubhamvishu merged 3 commits into
apache:mainfrom
shubhamvishu:native-private

Conversation

@shubhamvishu
Copy link
Copy Markdown
Contributor

Description

@shubhamvishu shubhamvishu added the skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check. label Mar 24, 2026
@shubhamvishu shubhamvishu requested a review from uschindler March 24, 2026 18:18
Copy link
Copy Markdown
Contributor

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

The slow code is at least working. It is a not of work.

For testing we can use a trick without native code: Maybe define java methods with correct signature with scalar impl and hook them in for testing. So method able to be called is just a mich deleting to a java class.

return (T) mh.invokeExact(args);
// TODO: This is slow and we should avoid dynamic invocations and improve the test coverage
// (https://github.com/apache/lucene/issues/15840)
return (T) mh.invokeWithArguments(args);
Copy link
Copy Markdown
Contributor

@uschindler uschindler Mar 24, 2026

Choose a reason for hiding this comment

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

I still think this should be different. Keep the invokeExact() here and convert the MethodHandle for foreign function using asSpreader(Object[].class, numberOfParams). That's a bit faster, but still has the varargs array creation and conversion of primitives to wrappers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Make sense, I switched it to back this invokeExact way

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may not work so easy, so let's do that later. We need tests. But in the ideal case we should call all methods statically. Its more try-catch logic but it is needed for performance.

@shubhamvishu shubhamvishu merged commit 1ccfbf8 into apache:main Mar 24, 2026
13 checks passed
@uschindler uschindler added this to the 11.0.0 milestone Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:core/other skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants