[code-reflection] RFR: Split OnnxRuntime into high-level and low-level generated code
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Feb 24 19:22:10 UTC 2025
On Thu, 20 Feb 2025 13:03:41 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
> This PR reorganizes the code in the OnnxRuntime class. It now contains an higher-level implementation of the onnx C API, which is separate from the low-level bindings. These bindings are defined in the new `foreign` package, and have been mechanically generated with a tweaked version of `jextract` which:
> * omits some of the code that is not required (e.g. struct getters/setters)
> * make the generated code more usable for onnx use case -- meaning each struct field should have an "invoker" accessor, which fetches the struct pointer and then invokes it
>
> The generated code is still quite big (OrtApi struct contains 10K LoC). I can make the bindings smaller if we only want to focus on the handful of functions we use today -- but in its current form, the low-level bindings are "complete" and support the full C API.
>
> The main changes in OnnxRuntime are as follows:
> * calls to function pointers in OrtApi/OrtApiBase now are handled as calls to static wrappers generated by jextract.
> * there's no need to try/catch and wrap exceptions because the generated code does that
> * some of the explicit reinterpret calls with an explicit size have been replaced by generated `reinterpret` methods (which use the correct struct layout size)
> * some of the constants (logging level, API version) are now taken from the generated code
>
> When doing this exercise, I uncovered an issue with the MHs for a couple of functions (`ReleaseEnv` and `ReleaseSession`) being incorrect -- the underlying function pointers for these are void (so no result) and they don't accept a trailing `ret` segment. Also calling `checkStatus` on them makes no sense (again, because these functions don't return a status).
I've added some changes to make sure that the segments returned by the high-level API keep the automatic arena reachable. There are two places where I've added a `reinterpret`:
* `retAddr` -- as the output parameter `ret` is accessed, a segment is read (which will have no arena)
* `run` -- this also features output parameters (`outputs`) so we need to keep the segments read from these output parameters connected to the automatic arena
-------------
PR Comment: https://git.openjdk.org/babylon/pull/323#issuecomment-2679426768
More information about the babylon-dev
mailing list