[Release] Cache Minja Template#4134
Open
apaniukov wants to merge 6 commits into
Open
Conversation
…encies group across 1 directory (openvinotoolkit#4107) Bumps the npm-dependencies group with 1 update in the /src/js directory: [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node). Updates `@types/node` from 24.13.2 to 24.13.3 <details> <summary>Commits</summary> <ul> <li>See full diff in <a href="https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node">compare view</a></li> </ul> </details> <br /> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
<!-- Keep your pull requests (PRs) as atomic as possible. That increases the likelihood that an individual PR won't be stuck because of adjacent problems, merge conflicts, or code review. Your merged PR is going to appear in the automatically generated release notes on GitHub. So the clearer the title the better. --> ## Description <!-- Please include a summary of the change. Also include relevant motivation and context. --> Skip NPU test failing in precommits: https://github.com/openvinotoolkit/openvino.genai/actions/runs/29002476348/job/86068988266?pr=4119 https://github.com/openvinotoolkit/openvino.genai/actions/runs/29005199890/job/86079240639 ## Checklist: - [x] This PR follows [GenAI Contributing guidelines](https://github.com/openvinotoolkit/openvino.genai?tab=contributing-ov-file#contributing). <!-- Always follow them. If there are deviations, explain what and why. --> - [x] Tests have been updated or added to cover the new code. <!-- Specify exactly which tests were added or updated. If the change isn't maintenance related, update the tests at https://github.com/openvinotoolkit/openvino.genai/tree/master/tests or explain in the description why the tests don't need an update. --> - [x] This PR fully addresses the ticket. <!--- If not, explain clearly what is covered and what is not. If follow-up pull requests are needed, specify in the description. --> - [NA] I have made corresponding changes to the documentation. <!-- Run github.com/\<username>/openvino.genai/actions/workflows/deploy_gh_pages.yml on your fork with your branch as a parameter to deploy a test version with the updated content. Replace this comment with the link to the built docs. If the documentation is updated in a separate PR, clearly specify it. -->
github-actions
Bot
added
the
category: tokenizers
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the “Cache Minja Chat Template” change to the release branch to avoid repeatedly parsing/constructing minja::chat_template objects during Tokenizer::apply_chat_template, significantly reducing chat-template application overhead in hot paths.
Changes:
- Added a small, mutex-protected cache of parsed
minja::chat_templateobjects keyed by the template string. - Updated
apply_chat_template()to reuse cached templates and to synchronize reads/updates of chat template state (set_chat_template(), getters). - Cleared the cache when
set_chat_template()updates the configured template to ensure correctness.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cpp/src/tokenizer/tokenizer_impl.hpp | Adds mutex + in-memory cache fields and declares the cached-template accessor. |
| src/cpp/src/tokenizer/tokenizer_impl.cpp | Implements the cache, switches apply_chat_template() to reuse cached templates, and synchronizes chat template state access. |
Comment on lines
+48
to
+49
| mutable std::mutex m_minja_chat_template_cache_mutex; | ||
| mutable std::unordered_map<std::string, std::shared_ptr<const minja::chat_template>> m_minja_chat_template_cache; |
apaniukov
requested review from
Retribution98,
Wovchena,
sgonorov and
yatarkan
as code owners
github-actions
Bot
added
category: LLM
|
|
||
| @pytest.mark.parametrize("llm_model", MODELS_LIST, indirect=True) | ||
| @pytest.mark.parametrize("npu_config", PIPELINE_CONFIGS, indirect=True) | ||
| @pytest.mark.xfail(reason="Error: KV-Cache is full: num_stored_tokens=319 capacity=319. Ticket 190518") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Port #4133 to release.
CVS-###
Fixes #(issue)
Checklist: