{{ message }}
Repository navigation
Skip serializing bulk edits when debug logging is off - #341765
Merged
Connor Peet (connor4312) merged 1 commit intoOct 15, 2026
Merged
Connor Peet (connor4312) merged 1 commit into
Connor Peet (connor4312) merged 1 commit into
Conversation
BulkEdit called JSON.stringify(edits) for every _perform*Edits call and passed the result to ILogService.debug, so the full edit list was serialized even when debug logging was disabled and the string was discarded. For Search "Replace All" this means one large stringify of every match on the UI thread. Only serialize when debug logging is on. Co-authored-by: wtfyd-agent[bot] <328989958+wtfyd-agent[bot]@users.noreply.github.com> Signed-off-by: Kevin Turcios <turcioskevinr@gmail.com>
Kevin Turcios (KRRT7)
force-pushed
the
bulkedit-skip-debug-serialization
branch
from
October 15, 2026 13:07
bef6fd0 to
7befad3
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow logging guard preserves existing behavior, and no actionable issues remain.
0 open findings
What changed in this PR
Avoids unnecessary bulk-edit serialization when debug logging is disabled, reducing overhead for operations such as Search Replace All.
Changes:
- Centralizes four logging calls in
_logEdits. - Guards serialization with
canLog, preserving enabled debug output and edit execution.
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Connor Peet (connor4312)
approved these changes
Oct 15, 2026
Connor Peet (connor4312)
enabled auto-merge
October 15, 2026 15:52
Nick Trogh (ntrogh)
approved these changes
Oct 15, 2026
Vritant Bhardwaj (vritant24)
approved these changes
Oct 15, 2026
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.
Every bulk edit serializes its full edit list with
JSON.stringify, even when debug logging is off. EachBulkEdit._perform*Editscall did:The string is built before
debug()checks the log level, so it is built and then thrown away at the defaultInfolevel. For Search Replace All that is one stringify of every match on the UI thread. On 10k matches, it was the largest JS self-time entry in the profile.This PR moves the four call sites into a
_logEditshelper that only serializes whencanLog(getLevel(), LogLevel.Debug)is true.canLogalso handlesLogLevel.Off. Log output is unchanged when debug logging is on.Measurements
The benchmark ran
BulkEditService.applywith realTextModels andModelService. Only the I/O and UI services were stubbed. Each Replace All was 10kfoo→baredits. Before and after were built frommain(02001a6) and this branch as identical bundles on both machines. Each process ran 10 warmup applies, then 50 measured ones. Before and after alternated each round, with 15 rounds per side.Instructions per apply (hardware counter, startup and warmup subtracted by differencing 0 vs 50 iterations):
perf,instructions:u)time -l)Wall-clock per apply (per-process median):
In all eight comparisons, every after run beat every before run. Mann–Whitney p ≤ 4e-05; the confidence intervals are bootstrap 95%. In CPU profiles, self time in
_performTextEditsdropped from 17–23% to about 0% on both machines. V8 attributes theJSON.stringifybuilt-in to its caller, so that is where the stringify cost showed up.These numbers cover only the bulk-edit step of Replace All, not the full end-to-end operation.
How to test
_performTextEditsline after Replace All.--log debug, Replace All still writes the[debug] _performTextEdits [...]line with every edit serialized (6 of 6 in a 3-file test) to the Window log.npm run typecheck-clientis clean.scripts/test.sh --grep "BulkEdit|BulkFileEdits|BulkCellEdits|MainThreadEditors|MainThreadBulkEdits"passes.