feat: add explicit encoding cache release - #605
mehrdadxzaker wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db67014385
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| encodings. Registered constructors and downloaded vocabulary files are kept. | ||
| """ | ||
| with _lock: | ||
| ENCODINGS.clear() |
There was a problem hiding this comment.
When clear_encoding_cache() runs concurrently with get_encoding() for an already cached name, this deletion can occur between the unlocked membership check and subscription at lines 67–68, causing the getter to raise an unexpected KeyError. Locking only the clear operation does not synchronize that reader; use a single atomic lookup with a sentinel or protect the fast path with the same lock.
Useful? React with 👍 / 👎.
1 participant
Encodings loaded through
get_encodingandencoding_for_modelare strongly cached for the lifetime of the process. Applications that finish a batch of tokenisation currently have no public way to release those cached references, even after dropping their own references and running garbage collection (#414).Add
tiktoken.clear_encoding_cache(). It clears the loaded-encoding cache under the same lock used for construction. Existing encodings and bound methods remain valid, while subsequent lookups construct and cache fresh instances. Registered constructors and downloaded vocabulary files are preserved, and ordinary lookup behavior is unchanged until callers explicitly clear the cache.Six offline regression tests cover collection of unused encodings, retained objects and bound methods, multiple encodings and repeated clears, clearing before the first lookup, pickle compatibility, and clearing while another thread constructs an encoding. The README includes a usage example and explains that freeing objects does not guarantee an immediate drop in RSS.
Addresses #414. This is an explicit cache-eviction API; it does not change the default cache policy.
Validation: native extension built from source on macOS arm64 / Python 3.12.2. All six new tests pass. Full suite: 38 passed, one pre-existing property-test failure (
test_hyp_roundtrip[cl100k_base]generates<|endofprompt|>, which defaultencode()rejects). Replayed that exact case against untouched upstream commit4e71bbeand confirmed the same failure. A three-cycle realo200k_baseload/clear probe confirmed the unused encoding was collected each time. No model API calls were used.Built both sdist and wheel, then installed the wheel into a fresh environment outside the checkout: all 10 selected registry, public API, and pickle tests pass.