Skip to content

fix: Mark the reversed() cache as valid after rebuilding it#61

Merged
spydon merged 1 commit into
mainfrom
fix/reversed-cache-never-valid
Jul 21, 2026
Merged

fix: Mark the reversed() cache as valid after rebuilding it#61
spydon merged 1 commit into
mainfrom
fix/reversed-cache-never-valid

Conversation

@spydon

@spydon spydon commented Jul 21, 2026

Copy link
Copy Markdown
Member

Description

_validReverseCache starts out true and is set to false on every mutation (add, remove, clear), but it was never set back to true when reversed() rebuilt the cache. As a result, after the first mutation every single reversed() call did a full toList(growable: false) of the set, and the "cache" never served a single hit.

This is a significant hidden cost for Flame, where Component.componentsAtLocation calls children.reversed() for every component with children on every pointer event, so every game pays an O(n) list materialization per node per hit test.

The fix marks the cache valid after rebuilding it. Invalidation behavior is unchanged: any mutation still resets the flag, and the existing reversed properly invalidates cache tests still pass. Added a regression test to both implementations asserting that the returned iterable is identical between calls when no mutation happened, and is correctly rebuilt after a mutation.

Found while working on flame-engine/flame#3957.

@github-actions

Copy link
Copy Markdown

Benchmark Results

Package ordered_set:

Benchmarks Current Branch
[fix/reversed-cache-never-valid]
Base Branch
[main]
Diff
Iteration Benchmark - Comparing 240.157 μs 249.320 μs 🟢 -3.675 %
Insertion and Removal Benchmark - Comparing 520.705 μs 522.828 μs 🟢 -0.406 %
Comprehensive Benchmark - Comparing 16226.411 μs 16327.258 μs 🟢 -0.618 %
Iteration Benchmark - Priority 258.616 μs 254.085 μs 🔴 +1.783 %
Insertion and Removal Benchmark - Priority 340.519 μs 341.264 μs 🟢 -0.218 %
Comprehensive Benchmark - Priority 15486.938 μs 15441.366 μs 🔴 +0.295 %

Benchmarks provided with 💙 by Dart Benchmark Action.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 29873276610

Coverage increased (+0.03%) to 97.537%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 2 of 2 lines across 2 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 203
Covered Lines: 198
Line Coverage: 97.54%
Coverage Strength: 3.07 hits per line

💛 - Coveralls

@spydon
spydon requested a review from luanpotter July 21, 2026 22:34

@luanpotter luanpotter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, shocked it was never set!

@spydon
spydon merged commit 552fa07 into main Jul 21, 2026
6 checks passed
@spydon
spydon deleted the fix/reversed-cache-never-valid branch July 21, 2026 22:36
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.

3 participants