Skip to content

[java] Adding native library loader to SessionOptions and RunOptions static init#16435

Merged
RyanUnderhill merged 1 commit intomicrosoft:mainfrom
Craigacp:session-options-static-init
Jul 3, 2023
Merged

[java] Adding native library loader to SessionOptions and RunOptions static init#16435
RyanUnderhill merged 1 commit intomicrosoft:mainfrom
Craigacp:session-options-static-init

Conversation

@Craigacp
Copy link
Copy Markdown
Contributor

Description

Unlike most ORT classes SessionOptions and RunOptions don't trigger native library loading of the JNI binding and ORT when the classes are initialized (after class loading). This was initially because I thought that loading an inner class would trigger the static initialization of the outer class, but this is not true. So if you create a SessionOptions instance before referencing OrtEnvironment then you won't trigger library loading and you'll get an error saying it couldn't link the native method that creates a SessionOptions object.

Note this doesn't prevent users from creating a SessionOptions and modifying it before the OrtEnvironment is created, which can still cause issues. It would be a breaking API change to modify the SessionOptions constructor to take an environment, and it wouldn't mirror the way it works in the C API which requires this by convention rather than API design, but we can discuss making that modification later.

Motivation and Context

Reduces the occurrence of mysterious Java library loading errors. Helps with #16434.

…e the native library is loaded before any methods are called.
@Craigacp Craigacp changed the title [java] Adding native library loader to SessionOptions and RunOptions init [java] Adding native library loader to SessionOptions and RunOptions static init Jun 20, 2023
@RyanUnderhill
Copy link
Copy Markdown
Contributor

/azp run Linux CPU CI Pipeline,Linux CPU x64 NoContribops CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,MacOS CI Pipeline,MacOS NoContribops CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline

@RyanUnderhill
Copy link
Copy Markdown
Contributor

/azp run Linux CPU Minimal Build E2E CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 6 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 7 pipeline(s).

@RyanUnderhill
Copy link
Copy Markdown
Contributor

/azp run Linux QNN CI Pipeline, ONNX Runtime Web CI Pipeline, Windows ARM64 QNN CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

@RyanUnderhill
Copy link
Copy Markdown
Contributor

/azp run orttraining-amd-gpu-ci-pipeline

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@RyanUnderhill
Copy link
Copy Markdown
Contributor

I think it's safe to squash and merge, the 1 failing test doesn't have anything to do with your changes.

@Craigacp
Copy link
Copy Markdown
Contributor Author

Craigacp commented Jul 3, 2023

Sounds good. Would you mind having a look at #16198 as well? That one is a bit more involved, but we'd really like to have better support for loading in large models without the filesystem from Java so I needed to add a few new native accessors.

@RyanUnderhill RyanUnderhill merged commit 13cc619 into microsoft:main Jul 3, 2023
@RyanUnderhill
Copy link
Copy Markdown
Contributor

Sounds good. Would you mind having a look at #16198 as well? That one is a bit more involved, but we'd really like to have better support for loading in large models without the filesystem from Java so I needed to add a few new native accessors.

Yep, will take a look

@Craigacp Craigacp deleted the session-options-static-init branch July 3, 2023 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants