perf: hoist attr accesses in _update_vscode_summary (#905)#907
perf: hoist attr accesses in _update_vscode_summary (#905)#907
Conversation
Bind _SummaryAccumulator dict fields and repeated VSCodeRequest
attributes to locals before the hot loop, converting LOAD_ATTR to
LOAD_FAST. Replace strftime("%Y-%m-%d") with date.isoformat() on the
already-computed date object.
Add TestUpdateVscodeSummaryLargeScale with 10 500 synthetic requests to
verify correctness-equivalence of the optimised loop (no wall-clock
timing).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Optimizes the hot aggregation loop in src/copilot_usage/vscode_parser.py::_update_vscode_summary, reducing repeated attribute lookups during per-request accumulation when parsing VS Code Copilot Chat logs.
Changes:
- Hoists accumulator dict attributes to locals and caches repeated
VSCodeRequestattribute reads inside the loop. - Replaces
strftime("%Y-%m-%d")withdate.isoformat()for date-key generation. - Adds a large-scale correctness test suite intended to validate the optimized loop’s results.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/copilot_usage/vscode_parser.py |
Refactors _update_vscode_summary to reduce attribute access overhead in the aggregation loop. |
tests/copilot_usage/test_vscode_parser.py |
Adds a new large-scale correctness-oriented test class for _update_vscode_summary. |
Replace weak per-date assertions (sum + format check) with exact expected mapping equality to properly verify the full date distribution from _update_vscode_summary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Low-impact performance optimization of a private internal function (_update_vscode_summary). Changes hoist attribute accesses to locals and replace strftime with isoformat — semantically equivalent, no API/contract changes. New TestUpdateVscodeSummaryLargeScale test class (3 methods, 10.5k synthetic requests) thoroughly validates correctness. All CI checks pass. Auto-approving for merge.
microsasa
deleted the
fix/905-hoist-vscode-summary-attrs-c5a6aa15a18dc89a
branch
Summary
Closes #905
Optimises the hot loop in
_update_vscode_summaryby:rbm,dbm,rbc,rbd) before the loop — convertsLOAD_ATTR(slot-descriptor protocol) toLOAD_FAST.req.*accesses (duration_ms,model,timestamp) to local variables within each iteration.strftime("%Y-%m-%d")withdate.isoformat()on the already-computeddateobject — avoids re-parsing the format string.For a log file with 5 000 requests this eliminates ~30 000
LOAD_ATTRops onaccand ~10 000 onreq.Testing
Added
TestUpdateVscodeSummaryLargeScalewith 10 500 syntheticVSCodeRequestobjects spanning multiple models, categories, and dates. Asserts bit-for-bit correctness of totals, per-model counts/durations, per-category counts, per-date counts, and timestamp bounds. No wall-clock timing — deterministic correctness only, matching the project's perf-test convention.