[jira] [Resolved] (OPENNLP-1844) Make opennlp-dl ONNX components (NameFinderDL, DocumentCategorizerDL, SentenceVectorsDL) thread-safe.

["Kristian Rickert (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/OPENNLP-1844?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Rickert resolved OPENNLP-1844.
---------------------------------------
    Resolution: Fixed

Merged into main

> Make opennlp-dl ONNX components (NameFinderDL, DocumentCategorizerDL, SentenceVectorsDL) thread-safe.
> -----------------------------------------------------------------------------------------------------
>
>                 Key: OPENNLP-1844
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1844
>             Project: OpenNLP
>          Issue Type: Improvement
>            Reporter: Kristian Rickert
>            Assignee: Kristian Rickert
>            Priority: Major
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> h2. Summary
> Make the {{opennlp-dl}} ONNX components ({*}{{{}NameFinderDL{}}}, {{{}DocumentCategorizerDL{}}}, {{SentenceVectorsDL}}{*}) thread-safe so a single instance - and its loaded *{{OrtSession}}* - can serve concurrent requests, instead of being externally synchronized or duplicated per thread. This mirrors the thread-safety work already done for the classic {{*ME}} components and is motivated by serving these models from multi-threaded services (e.g. a gRPC server), where one *{{OrtSession}}* per request thread is prohibitively expensive in memory and load time.
> h2. Background
> Inference in these components ({*}{{find}}{*}/{*}{{categorize}}{*}/{*}{{getVectors}}{*}) already holds no per-call instance state, and *{{ai.onnxruntime.OrtSession#run}}* supports concurrent execution. The gaps preventing a documented thread-safety guarantee were:
>  * {*}Safe publication{*}: the shared inference state in {{AbstractDL}} ({{{}env{}}}/{{{}session{}}}/{{{}tokenizer{}}}/{{{}vocab{}}}) was held in non-final fields assigned by each subclass constructor, so the Java Memory Model did not guarantee a fully constructed instance is visible to other threads.
>  * {*}Shared-resource lifecycle bug{*}: {{AbstractDL#close()}} closed the process-wide {{OrtEnvironment}} singleton returned by {{{}OrtEnvironment.getEnvironment(){}}}, which is shared by every DL component - closing one component tore down the environment others still depend on.
>  * No defensive copies of the {{ids2Labels}} / {{categories}} maps, allowing external
> mutation to race with inference.
> Unlike the {{ME}} classes, there is no per-call mutable state to isolate, so no {{ThreadLocal}} (or owner/fast-path mechanism) is required. The only shared state is the immutable, intentionally-shared {{{}OrtSession{}}}.
> h2. Changes
>  * *{{AbstractDL}}* initializes {{{}env{}}}/{{{}session{}}}/{{{}tokenizer{}}}/{{{}vocab{}}} once through a new {{protected AbstractDL(File, File, OrtSession.SessionOptions, boolean)}} constructor and marks them {{{}final{}}}, guaranteeing safe publication. Subclass constructors delegate via {{{}super(...){}}}.
>  * Added a {{*protected static OrtSession.SessionOptions* sessionOptions(InferenceOptions)}} helper to centralize CUDA/session-option setup.
>  * {{close()}} now closes only the {{OrtSession}} this instance owns; it no longer closes the shared {{{}OrtEnvironment{}}}.
>  * Defensive-copy {{ids2Labels}} and {{categories}} with {{{}Map.copyOf{}}}.
>  * {{loadVocab(...)}} and {{createTokenizer(...)}} are now {{static}} (they use no instance state).
>  * Annotated {{{}NameFinderDL{}}}, {{DocumentCategorizerDL}} and {{SentenceVectorsDL}} with {{{}@ThreadSafe{}}}. {{NameFinderDL}} is thread-safe provided the injected {{SentenceDetector}} is (e.g. {{{}SentenceDetectorME{}}}, which is {{{}@ThreadSafe{}}}).
> h2. Backward compatibility
> Public API of the components is unchanged - existing code that constructs the components and calls {{{}find{}}}/{{{}categorize{}}}/{{{}getVectors{}}} is fully source- and binary-compatible and now thread-safe.
> Two changes affect only code extending the internal {{AbstractDL}} base class:
>  # {{AbstractDL}} no longer has an implicit no-arg constructor; subclasses must delegate to the new {{protected}} constructor instead of assigning the (now {{{}final{}}}) fields directly.
>  # {{loadVocab}} / {{createTokenizer}} became {{{}static{}}}; source calls via an instance still compile, pre-compiled bytecode invoking them virtually needs a recompile.
> These are deliberate - making the shared state {{final}} is what guarantees safe publication - and acceptable in the 3.0 milestone line, where {{AbstractDL}} is an internal base with no external subclasses in the project.
> h2. Testing
>  * Existing {{opennlp-dl}} unit tests pass (updated {{CreateTokenizerTest}} and
> {{LoadVocabTest}} to use the now-static helpers).
>  * Added a concurrent {{NameFinderDL}} eval test ({{{}opennlp-eval-tests{}}}) that runs
> {{find()}} from multiple threads on one shared instance and asserts every result matches the single-threaded baseline; a data race would surface as a wrong span, an exception, or a non-deterministic result.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)