[code-reflection] RFR: Model lifetimes of onnx session-related objects more explicitly
Adam Sotona
asotona at openjdk.org
Mon Mar 3 12:11:35 UTC 2025
On Fri, 28 Feb 2025 12:42:24 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
> The class representing an onnx session is auto closeable. But, in the current code, a session is closed immediately after its `run` method is called. This is problematic because a session returns some ORTValues (tensors) which also need to be freed, but that cannot be freed immediately after calling `run` (as they need to be used by clients).
>
> To address this problem, I tweaked the session code to accept an external arena. All the allocation of session-related data structures now happens using that external arena. This means that the client can now be in charge of managing the lifetime of a session (see changes to MNIST demo).
>
> To test, I tweaked the MNIST code to do 10K iterations on each button pressed. Predictably, a single button pressed resulted in over 3g of memory being leaked. With these changes the memory arrives at ~400K (there is still some minor leak, but not sure worth pushing more).
>
> If the changes to the demo are not deemed good, I can withdraw this PR -- I mostly wanted to capture the result of my exploration somewhere.
Considering different use cases is this PR step forward, however further changes need deeper discussion. Letting the arena lifecycle on user is probably the best we can do now. I can imagine use cases where the model/session is used once and the resulting tensor suppose to live long outside of the try block. And I can also imagine a 100k of repeated execution of the same model/session, where session-long living results would cause significant memory leaks.
My the only remaining concern is related to the naming of Arena instance "onnxSession".
-------------
Marked as reviewed by asotona (Reviewer).
PR Review: https://git.openjdk.org/babylon/pull/332#pullrequestreview-2653948088
More information about the babylon-dev
mailing list