Skip to content

perf(transform): avoid redundant allocations when grouping events - #673

Open
0xbrayo wants to merge 1 commit into
ActivityWatch:masterfrom
0xbrayo:perf/group-events
Open

0xbrayo wants to merge 1 commit into
ActivityWatch:masterfrom
0xbrayo:perf/group-events

Conversation

@0xbrayo

@0xbrayo 0xbrayo commented Sep 11, 2026

Copy link
Copy Markdown
Member

Grouping events currently allocates an unused map for each new group, clones the first event's JSON, and performs redundant hash lookups. Use HashMap::entry and move that event into the group, preserving the first timestamp and payload, clearing its ID, and accumulating later durations.

Part of #671 (grouping allocations).

Validation:

  • cargo test -p aw-transform: 50 tests passed.
  • Added a regression for multiple keys, missing keys, nested payloads, first-event metadata, and ID clearing.
  • Added Criterion cases for 50,000 events with 100 and 50,000 distinct groups, with input setup outside the timed operation. Local optimized timings were approximately 9.4 ms and 8.9 ms respectively. An earlier same-process prototype comparison measured 22.6 ms → 10.3 ms for the all-distinct case; these are synthetic measurements, not application-wide speedups.

This retains the existing grouping-key encoding and unordered output.

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule violations.

Summary

  • Uses HashMap::entry to avoid redundant lookups.
  • Moves the first event into each group instead of cloning its payload.
  • Clears the retained event ID and accumulates subsequent durations.
  • Adds regression coverage and Criterion benchmarks for low- and high-cardinality grouping.

Reviews (2) · Last reviewed commit: "perf(transform): avoid redundant allocat..."

Comment thread aw-transform/src/merge.rs
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.74%. Comparing base (656f3c9) to head (8b7d519).
⚠️ Report is 103 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #673      +/-   ##
==========================================
+ Coverage   70.81%   79.74%   +8.93%     
==========================================
  Files          51       67      +16     
  Lines        2916     5851    +2935     
==========================================
+ Hits         2065     4666    +2601     
- Misses        851     1185     +334     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@0xbrayo

0xbrayo commented Sep 13, 2026

Copy link
Copy Markdown
Member Author

@greptile review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant