[
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)
