[I] DynMethods.UnboundMethod.invokeChecked copies the argument array when the argument count already matches [kyuubi]

["LuciferYang (via GitHub)" <gi...@apache.org>]

LuciferYang opened a new issue, #7699:
URL: https://github.com/apache/kyuubi/issues/7699

   ### Description
   
   `DynMethods.UnboundMethod.invokeChecked` branches only on whether the matched method is varargs:
   
   ```java
           if (argLength < 0) {
             return (R) method.invoke(target, args);
           } else {
             return (R) method.invoke(target, Arrays.copyOfRange(args, 0, argLength));
           }
   ```
   
   For any fixed-arity method it copies the argument array, including when `args.length` already equals `argLength`. There the copy carries the same element references into `Method.invoke`, and `Arrays.copyOfRange(T[], int, int)` preserves the source array's runtime component type, so it is not normalizing the carrier's type either. It is discarded as soon as the call returns.
   
   The copy exists to change arity: to drop trailing arguments when the caller passes more than the matched method takes, and to null-pad when it passes fewer. When the counts are equal neither applies, and the varargs branch already passes the caller's array straight through.
   
   So every exact-arity call allocates one array for no effect. Nobody has measured how hot the affected call sites are.
   
   Two behaviours of the copying branch are relied on and have no test:
   
   `HiveConnectorUtils.newStorageFormat` and `copyStorageFormat` invoke with 7 arguments against the 6-argument `CatalogStorageFormat.apply` that Spark below 4.2 provides, and depend on the trailing `serdeName` being dropped. A comment above that call site spells the dependency out. `DynMethodsTest` covers exact arity and varargs only, so nothing pins it.
   
   A caller passing fewer arguments than the matched method takes reaches the target with trailing nulls rather than being rejected. Also untested, and it constrains any change to this branch.
   
   ### Affects Version(s)
   
   master (1.13.0-SNAPSHOT). `DynMethods` is copied from parquet-common by way of iceberg-common, and both upstreams still copy unconditionally, so this is not a regression from a sync.
   
   ### Are you willing to submit PR?
   
   Yes.
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

Re: [I] DynMethods.UnboundMethod.invokeChecked copies the argument array when the argument count already matches [kyuubi]

["pan3793 (via GitHub)" <gi...@apache.org>]

pan3793 closed issue #7699: DynMethods.UnboundMethod.invokeChecked copies the argument array when the argument count already matches
URL: https://github.com/apache/kyuubi/issues/7699


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org