[C++][Gandiva] Support external function registry #37753
Comments
|
IMO this is definitely a useful feature. You are very welcome to submit a PR if you would like to! |
|
Related mailing list discussion: https://lists.apache.org/thread/lm4sbw61w9cl7fsmo7tz3gvkq0ox6rod |
kou
changed the title
kou
added a commit
that referenced
this issue
Nov 8, 2023
# Rationale for this change This PR tries to enhance Gandiva by supporting external function registry, so that developers can author third party functions without modifying Gandiva's core codebase. See #37753 for more details. In this PR, the external function needs to be compiled into LLVM IR for integration. # What changes are included in this PR? Two new APIs are added to `FunctionRegistry`: ```C++ /// \brief register a set of functions into the function registry from a given bitcode /// file arrow::Status Register(const std::vector<NativeFunction>& funcs, const std::string& bitcode_path); /// \brief register a set of functions into the function registry from a given bitcode /// buffer arrow::Status Register(const std::vector<NativeFunction>& funcs, std::shared_ptr<arrow::Buffer> bitcode_buffer); ``` Developers can use these two APIs to register external functions. Typically, developers will register a set of function metadatas (`funcs`) for all functions in a LLVM bitcode file, by giving either the path to the LLVM bitcode file or an `arrow::Buffer` containing the LLVM bitcode buffer. The overall flow looks like this:  # Are these changes tested? Some unit tests are added to verify this enhancement # Are there any user-facing changes? Some new ways to interfacing the library are added in this PR: * The `Configuration` class now supports accepting a customized function registry, which developers can register their own external functions and uses it as the function registry * The `FunctionRegistry` class has two new APIs mentioned above * The `FunctionRegistry` class, after instantiation, now it doesn't have any built-in function registered in it. And we switch to use a new function `GANDIVA_EXPORT std::shared_ptr<FunctionRegistry> default_function_registry();` to retrieve the default function registry, which contains all the Gandiva built-in functions. * Some library depending on Gandiva C++ library, such as Gandiva's Ruby binding's `Gandiva::FunctionRegistry` class behavior is changed accordingly # Notes * Performance * the code generation time grows with the number of externally added function bitcodes (the more functions are added, the slower the codegen will be), even if the externally function is not used in the given expression at all. But this is not a new issue, and it applies to built-in functions as well (the more built-in functions are there, the slower the codegen will be). In my limited testing, this is because `llvm::Linker::linkModule` takes non trivial of time, which happens to every IR loaded, and the `RemoveUnusedFunctions` happens after that, which doesn't help to reduce the time of `linkModule`. We may have to selectively load only necessary IR (primarily selectively doing `linkModule` for these IR), but more metadata may be needed to tell which functions can be found in which IR. This could be a separated PR for improving it, please advice if any one has any idea on improving it. Thanks. * Integration with other programming languages via LLVM IR/bitcode * So far I only added an external C++ function in the codebase for unit testing purpose. Rust based function is possible but I gave it a try and found another issue (Rust has std lib which needs to be processed in different approach), I will do some exploration for other languages such as zig later. * Non pre-compiled functions, may require some different approach to get the function pointer, and we may discuss and work on it in a separated PR later. Another issue #38589 was logged for this. * The discussion thread in dev mail list, https://lists.apache.org/thread/lm4sbw61w9cl7fsmo7tz3gvkq0ox6rod * I submitted another PR previously (#37787) which introduced JSON based function registry, and after discussion, I will close that PR and use this PR instead * Closes: #37753 Lead-authored-by: Yue Ni <niyue.com@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
JerAguilon
pushed a commit
to JerAguilon/arrow
that referenced
this issue
Nov 9, 2023
…apache#38116) # Rationale for this change This PR tries to enhance Gandiva by supporting external function registry, so that developers can author third party functions without modifying Gandiva's core codebase. See apache#37753 for more details. In this PR, the external function needs to be compiled into LLVM IR for integration. # What changes are included in this PR? Two new APIs are added to `FunctionRegistry`: ```C++ /// \brief register a set of functions into the function registry from a given bitcode /// file arrow::Status Register(const std::vector<NativeFunction>& funcs, const std::string& bitcode_path); /// \brief register a set of functions into the function registry from a given bitcode /// buffer arrow::Status Register(const std::vector<NativeFunction>& funcs, std::shared_ptr<arrow::Buffer> bitcode_buffer); ``` Developers can use these two APIs to register external functions. Typically, developers will register a set of function metadatas (`funcs`) for all functions in a LLVM bitcode file, by giving either the path to the LLVM bitcode file or an `arrow::Buffer` containing the LLVM bitcode buffer. The overall flow looks like this:  # Are these changes tested? Some unit tests are added to verify this enhancement # Are there any user-facing changes? Some new ways to interfacing the library are added in this PR: * The `Configuration` class now supports accepting a customized function registry, which developers can register their own external functions and uses it as the function registry * The `FunctionRegistry` class has two new APIs mentioned above * The `FunctionRegistry` class, after instantiation, now it doesn't have any built-in function registered in it. And we switch to use a new function `GANDIVA_EXPORT std::shared_ptr<FunctionRegistry> default_function_registry();` to retrieve the default function registry, which contains all the Gandiva built-in functions. * Some library depending on Gandiva C++ library, such as Gandiva's Ruby binding's `Gandiva::FunctionRegistry` class behavior is changed accordingly # Notes * Performance * the code generation time grows with the number of externally added function bitcodes (the more functions are added, the slower the codegen will be), even if the externally function is not used in the given expression at all. But this is not a new issue, and it applies to built-in functions as well (the more built-in functions are there, the slower the codegen will be). In my limited testing, this is because `llvm::Linker::linkModule` takes non trivial of time, which happens to every IR loaded, and the `RemoveUnusedFunctions` happens after that, which doesn't help to reduce the time of `linkModule`. We may have to selectively load only necessary IR (primarily selectively doing `linkModule` for these IR), but more metadata may be needed to tell which functions can be found in which IR. This could be a separated PR for improving it, please advice if any one has any idea on improving it. Thanks. * Integration with other programming languages via LLVM IR/bitcode * So far I only added an external C++ function in the codebase for unit testing purpose. Rust based function is possible but I gave it a try and found another issue (Rust has std lib which needs to be processed in different approach), I will do some exploration for other languages such as zig later. * Non pre-compiled functions, may require some different approach to get the function pointer, and we may discuss and work on it in a separated PR later. Another issue apache#38589 was logged for this. * The discussion thread in dev mail list, https://lists.apache.org/thread/lm4sbw61w9cl7fsmo7tz3gvkq0ox6rod * I submitted another PR previously (apache#37787) which introduced JSON based function registry, and after discussion, I will close that PR and use this PR instead * Closes: apache#37753 Lead-authored-by: Yue Ni <niyue.com@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this issue
Nov 13, 2023
…apache#38116) # Rationale for this change This PR tries to enhance Gandiva by supporting external function registry, so that developers can author third party functions without modifying Gandiva's core codebase. See apache#37753 for more details. In this PR, the external function needs to be compiled into LLVM IR for integration. # What changes are included in this PR? Two new APIs are added to `FunctionRegistry`: ```C++ /// \brief register a set of functions into the function registry from a given bitcode /// file arrow::Status Register(const std::vector<NativeFunction>& funcs, const std::string& bitcode_path); /// \brief register a set of functions into the function registry from a given bitcode /// buffer arrow::Status Register(const std::vector<NativeFunction>& funcs, std::shared_ptr<arrow::Buffer> bitcode_buffer); ``` Developers can use these two APIs to register external functions. Typically, developers will register a set of function metadatas (`funcs`) for all functions in a LLVM bitcode file, by giving either the path to the LLVM bitcode file or an `arrow::Buffer` containing the LLVM bitcode buffer. The overall flow looks like this:  # Are these changes tested? Some unit tests are added to verify this enhancement # Are there any user-facing changes? Some new ways to interfacing the library are added in this PR: * The `Configuration` class now supports accepting a customized function registry, which developers can register their own external functions and uses it as the function registry * The `FunctionRegistry` class has two new APIs mentioned above * The `FunctionRegistry` class, after instantiation, now it doesn't have any built-in function registered in it. And we switch to use a new function `GANDIVA_EXPORT std::shared_ptr<FunctionRegistry> default_function_registry();` to retrieve the default function registry, which contains all the Gandiva built-in functions. * Some library depending on Gandiva C++ library, such as Gandiva's Ruby binding's `Gandiva::FunctionRegistry` class behavior is changed accordingly # Notes * Performance * the code generation time grows with the number of externally added function bitcodes (the more functions are added, the slower the codegen will be), even if the externally function is not used in the given expression at all. But this is not a new issue, and it applies to built-in functions as well (the more built-in functions are there, the slower the codegen will be). In my limited testing, this is because `llvm::Linker::linkModule` takes non trivial of time, which happens to every IR loaded, and the `RemoveUnusedFunctions` happens after that, which doesn't help to reduce the time of `linkModule`. We may have to selectively load only necessary IR (primarily selectively doing `linkModule` for these IR), but more metadata may be needed to tell which functions can be found in which IR. This could be a separated PR for improving it, please advice if any one has any idea on improving it. Thanks. * Integration with other programming languages via LLVM IR/bitcode * So far I only added an external C++ function in the codebase for unit testing purpose. Rust based function is possible but I gave it a try and found another issue (Rust has std lib which needs to be processed in different approach), I will do some exploration for other languages such as zig later. * Non pre-compiled functions, may require some different approach to get the function pointer, and we may discuss and work on it in a separated PR later. Another issue apache#38589 was logged for this. * The discussion thread in dev mail list, https://lists.apache.org/thread/lm4sbw61w9cl7fsmo7tz3gvkq0ox6rod * I submitted another PR previously (apache#37787) which introduced JSON based function registry, and after discussion, I will close that PR and use this PR instead * Closes: apache#37753 Lead-authored-by: Yue Ni <niyue.com@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Closed
dgreiss
pushed a commit
to dgreiss/arrow
that referenced
this issue
Feb 19, 2024
…apache#38116) # Rationale for this change This PR tries to enhance Gandiva by supporting external function registry, so that developers can author third party functions without modifying Gandiva's core codebase. See apache#37753 for more details. In this PR, the external function needs to be compiled into LLVM IR for integration. # What changes are included in this PR? Two new APIs are added to `FunctionRegistry`: ```C++ /// \brief register a set of functions into the function registry from a given bitcode /// file arrow::Status Register(const std::vector<NativeFunction>& funcs, const std::string& bitcode_path); /// \brief register a set of functions into the function registry from a given bitcode /// buffer arrow::Status Register(const std::vector<NativeFunction>& funcs, std::shared_ptr<arrow::Buffer> bitcode_buffer); ``` Developers can use these two APIs to register external functions. Typically, developers will register a set of function metadatas (`funcs`) for all functions in a LLVM bitcode file, by giving either the path to the LLVM bitcode file or an `arrow::Buffer` containing the LLVM bitcode buffer. The overall flow looks like this:  # Are these changes tested? Some unit tests are added to verify this enhancement # Are there any user-facing changes? Some new ways to interfacing the library are added in this PR: * The `Configuration` class now supports accepting a customized function registry, which developers can register their own external functions and uses it as the function registry * The `FunctionRegistry` class has two new APIs mentioned above * The `FunctionRegistry` class, after instantiation, now it doesn't have any built-in function registered in it. And we switch to use a new function `GANDIVA_EXPORT std::shared_ptr<FunctionRegistry> default_function_registry();` to retrieve the default function registry, which contains all the Gandiva built-in functions. * Some library depending on Gandiva C++ library, such as Gandiva's Ruby binding's `Gandiva::FunctionRegistry` class behavior is changed accordingly # Notes * Performance * the code generation time grows with the number of externally added function bitcodes (the more functions are added, the slower the codegen will be), even if the externally function is not used in the given expression at all. But this is not a new issue, and it applies to built-in functions as well (the more built-in functions are there, the slower the codegen will be). In my limited testing, this is because `llvm::Linker::linkModule` takes non trivial of time, which happens to every IR loaded, and the `RemoveUnusedFunctions` happens after that, which doesn't help to reduce the time of `linkModule`. We may have to selectively load only necessary IR (primarily selectively doing `linkModule` for these IR), but more metadata may be needed to tell which functions can be found in which IR. This could be a separated PR for improving it, please advice if any one has any idea on improving it. Thanks. * Integration with other programming languages via LLVM IR/bitcode * So far I only added an external C++ function in the codebase for unit testing purpose. Rust based function is possible but I gave it a try and found another issue (Rust has std lib which needs to be processed in different approach), I will do some exploration for other languages such as zig later. * Non pre-compiled functions, may require some different approach to get the function pointer, and we may discuss and work on it in a separated PR later. Another issue apache#38589 was logged for this. * The discussion thread in dev mail list, https://lists.apache.org/thread/lm4sbw61w9cl7fsmo7tz3gvkq0ox6rod * I submitted another PR previously (apache#37787) which introduced JSON based function registry, and after discussion, I will close that PR and use this PR instead * Closes: apache#37753 Lead-authored-by: Yue Ni <niyue.com@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
Description
Our team has been leveraging Gandiva in our projects, and its performance and capabilities have been commendable. However, we've identified a constraint concerning the registration of functions. At present, Gandiva necessitates that functions be registered directly within its codebase. This method, while functional, is not the most user-friendly and presents hurdles for those aiming to incorporate third-party functions. Direct modifications to Gandiva's source code for such integrations can inadvertently introduce maintenance challenges and potential versioning conflicts down the line.
Proposal
To address this limitation, I propose the introduction of an external function registry mechanism in Gandiva. This would allow users and developers to register and integrate custom functions without directly modifying Gandiva's core source code.
Two major changes are proposed for supporting this capability:
AddFunction(NativeFunction native_function)forFunctionRegistry, where the given parameternative_functionstores the external function metadata, so that developers can register external functions by calling this API. The external function metadata discovery responsibility is outside the scope of this proposal and developers can come up with her ownI've came up with a PR (#37787) with more details, including the JSON schema and a minimum external C++ external function registered via this approach to demonstrate/test how it works.The latest PR for implementing this proposal is here (#38116)
Benefits
While the intricate design specifics are still under consideration, I'm keen to understand the community's perspective on this proposal. Would an external function registry align with Gandiva's future roadmap? Your insights would be invaluable. Thank you.
Component(s)
C++ - Gandiva
UPDATE:
The text was updated successfully, but these errors were encountered: