Remove cache eviction system#766
Merged
Julesssss merged 6 commits intoExpensify:mainfrom Apr 10, 2026
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 577832e3eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
fabioh8010
commented
Apr 10, 2026
Comment on lines
+1069
to
+1071
| // This first .then() adds a microtask tick for compatibility reasons and | ||
| // to ensure subscribers don't receive an extra initial callback before Onyx.update() data arrives. | ||
| .then(() => undefined) |
Contributor
Author
There was a problem hiding this comment.
@Julesssss I made this change because of this comment from Codex. I tried to remove this then but it make some tests fails and generally I don't think it's safe to push with this PR. I would like to explore this in a follow-up if you don't mind.
roryabraham
approved these changes
Apr 10, 2026
Julesssss
approved these changes
Apr 10, 2026
Contributor
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.
Details
This PR is a follow-up of #752. As discussed here, we decided to keep these changes in this separate PR.
Before Onyx had two separate eviction systems sharing infrastructure:
Cache eviction — trimmed the in-memory
storageMapwhenmaxCachedKeysCountwas exceeded, triggered every time a subscriber connected to an evictable key viaaddKeyToRecentlyAccessedIfNeeded→removeLeastRecentlyUsedKeys. UsedrecentKeys(LRU of all accessed keys) andmaxRecentKeysSize.Storage eviction — freed device storage on capacity errors (quota exceeded, disk full) by evicting the least recently accessed evictable key via
retryOperation→getKeyForEviction. UsedrecentlyAccessedKeys(LRU of evictable keys only) andevictionBlocklist(controlled byuseOnyx'scanEvictoption).This PR removes cache eviction (1) entirely and simplifies storage eviction (2) by removing the
canEvict/blocklist mechanism, while keeping the core storage eviction flow intact. We are also fixing this issue reported some time ago by @VickyStash with this small commit.In my investigations about storage errors, there were only 895 cases where we could possibly hit the
canEvict/blocklist mechanism out of 3364645 errors analysed during a span of 30 days, representing only 0.027% of the cases. Because of this it's reasonable to remove this part and simplify furtheruseOnyxand Onyx internals code.Related Issues
$ Expensify/App#85252
E/App PR: Expensify/App#86736
Automated Tests
Tests related to cache eviction were removed and new ones were added/changed for storage eviction.
Manual Tests
We don't have much to test here since we are basically removing "dead code", so let's do a basic storage write test in a heavy account. Use Expensify/App#86736 for testing:
Author Checklist
### Related Issuessection aboveTestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-04-10.at.09.29.57-compressed.mov
Android: mWeb Chrome
N/A
iOS: Native
Screen.Recording.2026-04-10.at.09.35.08-compressed.mov
iOS: mWeb Safari
Screen.Recording.2026-04-10.at.09.38.19-compressed.mov
MacOS: Chrome / Safari
Screen.Recording.2026-04-10.at.09.42.54-compressed.mov
Screen.Recording.2026-04-10.at.09.45.10-compressed.mov