Repository navigation
perf(workbench): avoid copying unused dictionary items - #1661
Draft
mldangelo-oai wants to merge 1 commit into
Draft
mldangelo-oai wants to merge 1 commit into
mldangelo-oai wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Summary
Dictionary previews create a list of every item before selecting the first 20. Use
itertools.isliceto enforce the existing limit without allocating that temporary list.Changes
isliceand use it for the dictionary preview loop.One production file changes. There is no net line-count change or dependency change.
Testing
Opened as a draft: local validation and independent review are incomplete.
git diff --check: passed.node --experimental-strip-types .github/scripts/check_plugin_source_compatibility.mts: passed against the complete source checker. This does not replace the required compiled invocation.node sdk/typescript/scripts/check-plugin-source.mjs: passed.Risk and rollout
The production caller projects freshly decoded JSON dictionaries. For that path, the change retains the first-20-item order and projection behavior, including invalid-Unicode handling. It changes no CLI arguments, response schemas, persisted data or resource limit.
Custom dictionary subclasses with side effects during iteration, or concurrent mutation, could observe different behavior; the identified production path does not supply those objects. The benefit is avoiding an unnecessary allocation, not a claimed production benchmark improvement. Complete the required validation and review before merging.
Public disclosure review