Skip to content

Make tokenizer per-thread to unblock concurrent Redact - #8

Merged
ericstj merged 2 commits into
mainfrom
tokenizer-per-thread
Jul 24, 2026
Merged

Make tokenizer per-thread to unblock concurrent Redact#8
ericstj merged 2 commits into
mainfrom
tokenizer-per-thread

Conversation

@ericstj

@ericstj ericstj commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

PrivacyFilterTokenizer serialized all tokenization behind a global lock, which bottlenecked concurrent Redact calls on a single shared PrivacyFilter — even though ONNX Runtime's Run is already thread-safe. The tokenizer was the last serialization point preventing a shared filter from serving many concurrent redactions at full throughput.

This gives each thread its own TiktokenTokenizer via ThreadLocal<T> instead of locking (the tokenizer keeps internal encode caches that aren't guaranteed safe for concurrent mutation), and disposes it with the filter. Encode logic is otherwise unchanged.

Why not a batch API?

Concurrency beats batching for this model: a padded batch does maxLen work for every row, so throughput comes from driving many concurrent single-string Run calls on one shared instance — which this unblocks — not from a batch entrypoint. The single-string API is unchanged.

Testing

  • New EncodeIsThreadSafeUnderConcurrency test hammers a shared tokenizer from all cores (256 iterations) against the tokenizer oracle; mismatched ids/offsets or an exception would reveal unsafe shared state.
  • Full suite: 12/12 passing.

PrivacyFilterTokenizer serialized all tokenization behind a global lock, bottlenecking concurrent Redact calls on a single shared PrivacyFilter even though ONNX Runtime's Run is thread-safe. TiktokenTokenizer keeps internal encode caches that aren't guaranteed safe for concurrent mutation, so give each thread its own instance (ThreadLocal) instead of locking, and dispose it with the filter.

Adds a concurrency test that hammers a shared tokenizer from many threads against the oracle.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 617e892b-0037-40a1-b8ff-ce4c08b8e051
Copilot AI review requested due to automatic review settings July 24, 2026 03:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the global tokenizer lock that serialized all tokenization work, enabling a single shared PrivacyFilter instance to handle many concurrent Redact calls without bottlenecking on tokenization.

Changes:

  • Replaced global locking in PrivacyFilterTokenizer with a per-thread TiktokenTokenizer via ThreadLocal<T>.
  • Ensured PrivacyFilter disposes its tokenizer during Dispose().
  • Added a concurrency stress test to validate Encode behavior under parallel usage.
Show a summary per file
File Description
tests/PrivacyFilter.Net.Tests/TokenizerTests.cs Adds a parallel stress test to validate thread-safe Encode behavior against the oracle.
src/PrivacyFilter.Net/PrivacyFilterTokenizer.cs Switches to a per-thread tokenizer instance and adds IDisposable to clean up the ThreadLocal.
src/PrivacyFilter.Net/PrivacyFilter.cs Disposes the tokenizer when the filter instance is disposed.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment thread tests/PrivacyFilter.Net.Tests/TokenizerTests.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 03:44
@ericstj
ericstj merged commit 33f4ee1 into main Jul 24, 2026
3 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Comments suppressed due to low confidence (1)

tests/PrivacyFilter.Net.Tests/TokenizerTests.cs:33

  • PrivacyFilterTokenizer now implements IDisposable, but MatchesTiktokenOracle still constructs it without disposing. This can leave the internal ThreadLocal<T> undisposed across the test run; switch this test to using var as well for symmetry with the new concurrency test.
    [Fact]
    public void EncodeIsThreadSafeUnderConcurrency()
    {
        // The tokenizer is now per-thread (replacing a global lock), so a single
        // shared instance must serve concurrent Encode calls without corruption.
        // Hammer it from many threads against the oracle: mismatched ids/offsets or
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +54 to +58
Assert.Equal(testCase.Ids, actual.TokenIds);
Assert.Equal(testCase.Decoded, actual.DecodedText);
Assert.Equal(testCase.Starts, actual.CharacterStarts);
Assert.Equal(testCase.Ends, actual.CharacterEnds);
}
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