andygrove opened a new issue, #5252: URL: https://github.com/apache/datafusion-comet/issues/5252
Follow-up from review of #4459 ([thread](https://github.com/apache/datafusion-comet/pull/4459#discussion_r2601290186)). `ImportedCScalarUdf` holds its kernel as `Mutex<Box<CometCScalarKernel>>`. Every `return_type` and `invoke_with_args` takes that lock, so all concurrent batches for a given UDF in a process serialize on one mutex, even though the kernel is logically immutable after load and the per-execution state already lives in a separate `CometCScalarKernelImpl` built fresh per call. The lock is defensive rather than required: the comment notes DataFusion serializes invocations of a given `ScalarUDFImpl` anyway, and the FFI `Drop` is what is not `Sync`-safe. @paleolimbot suggested `Arc<CometCScalarKernel>` would maintain the reference counts and release the instance correctly when the last reference goes. Worth evaluating, along with simply holding `Box<CometCScalarKernel>` directly given the struct is already `Send + Sync` and the kernel is only read after load. Whatever shape this takes, it should preserve the property that the kernel's `release` runs exactly once, and it should not outlive the `LoadedLibrary` that dlopened it (see the field ordering in `LoadedLibrary`). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
