You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "shubhamvishu (via GitHub)" <gi...@apache.org> on 2026/03/24 18:13:39 UTC
[PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
shubhamvishu opened a new pull request, #15866:
URL: https://github.com/apache/lucene/pull/15866
### Description
- Addresses some comments on [#15508](https://github.com/apache/lucene/pull/15508) to make some classes package private for consistency and a minor fix to method name (we'll eventually planning to remove it in https://github.com/apache/lucene/issues/15840).
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984105309
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,7 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- return (T) mh.invokeExact(args);
+ return (T) mh.asSpreader(Object[].class, args.length).invokeExact(args);
Review Comment:
I mean keep invokeWithArguments() for now until we have tests.
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984088232
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,7 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- return (T) mh.invokeExact(args);
+ return (T) mh.asSpreader(Object[].class, args.length).invokeExact(args);
Review Comment:
nah revert that, this does not help. The idea was to transform the methodhandles once directly after lookup (above) and call them here with invokeExact().
But I am not sure if this works correct, without test, as the type casting may not work with an Object[] array. So let's keep the previous code. Sorry.
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984116055
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,9 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- 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);
Review Comment:
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.
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984103782
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,7 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- return (T) mh.invokeExact(args);
+ return (T) mh.asSpreader(Object[].class, args.length).invokeExact(args);
Review Comment:
Ahh OK I see what you mean now. Yeah, I'll revert. I'll try to shoot this down with the testing related change(as followup).
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984020731
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,9 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- 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);
Review Comment:
I still think this should be different. Keep the invokeExact() here and convert the MethodHandle for delegate and 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.
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu merged PR #15866:
URL: https://github.com/apache/lucene/pull/15866
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984020731
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,9 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- 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);
Review Comment:
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.
--
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
Re: [PR] Make some native vectorization classes package private to be consistent and a minor fix [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu commented on code in PR #15866:
URL: https://github.com/apache/lucene/pull/15866#discussion_r2984076499
##########
lucene/core/src/java25/org/apache/lucene/internal/vectorization/NativeVectorUtilSupport.java:
##########
@@ -259,7 +259,9 @@ private static float invokeFloatMethodHandle(MethodHandle mh, MemorySegment a, M
private static <T> T invokeOrDelegate(MethodHandle mh, Supplier<T> delegate, Object... args) {
if (mh != null) {
try {
- 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);
Review Comment:
Make sense, I switched it to back this invokeExact way
--
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