[macOS] Lazy-load favicon images to reduce memory by ~316 MB#4775
Draft
diegoreymendez wants to merge 1 commit into
Draft
[macOS] Lazy-load favicon images to reduce memory by ~316 MB#4775diegoreymendez wants to merge 1 commit into
diegoreymendez wants to merge 1 commit into
Conversation
FaviconStore.loadFavicons() eagerly fetched every favicon row at launch with returnsObjectsAsFaults=false and decoded each NSImage via EncryptedValueTransformer. Memory profiling traced ~310 MB of resident memory to this single subsystem. Replace the eager [URL: Favicon] dictionary in FaviconImageCache with: - entries: [URL: FaviconMetadata] — lightweight metadata, eagerly loaded at launch via loadFaviconMetadata() that never touches imageEncrypted. - imageCache: NSCache<NSURL, NSImage> with a 32 MB cost limit, populated on demand via a new sync loadImage(for:) that does a single-row CoreData fetch by identifier. cleanOld / burn / burnDomains continue to filter on documentUrl.host and dateCreated, both available on the metadata. FaviconsTabExtension subscribes to faviconsLoadedPublisher so pinned tabs re-resolve their favicon once metadata finishes loading at cold start. Measured on the macOS debug build: - Footprint: 655 MB → 339 MB (-316 MB, -48%) - MALLOC_LARGE: 432 MB → 129 MB
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.
Task/Issue URL: https://app.asana.com/1/137249556945/task/1214612370091414?focus=true
Description
The macOS browser was holding every saved favicon's bitmap in memory for the lifetime of the process — roughly 310 MB of resident memory traceable to the favicon load path. This change keeps lightweight favicon metadata in memory and lazy-loads images on demand, dropping steady-state footprint by about 48% with no behavioural regression for the UIs that show favicons before their pages have loaded.
Testing Steps
Impact and Risks
What could go wrong?
The main risk is favicons not appearing for users where we expect them to. The most likely failure mode is a cold-start race where pinned tabs render before the metadata cache loads, addressed by re-resolving once the cache is ready. Manage Bookmarks scrolled over a very long list may pay a brief synchronous I/O cost on first reveal as the image cache warms; subsequent scrolls are smooth.
Quality Considerations
Memory was measured with footprint and vmmap before and after. Steady-state footprint dropped from 655 MB to 339 MB; large allocations dropped from 432 MB to 129 MB. Existing favicon-related unit tests pass. Targeted unit tests for the new metadata-fetch and cache-miss flow are a follow-up.
Notes to Reviewer
The cache-miss path is synchronous on the main thread to keep the diff contained and avoid rippling consumer-side changes. If anyone reports stutters during burn or heavy bookmark imports we can switch to an async-with-placeholder pattern without changing the public surface.
Internal references:
Definition of Done | Engineering Expectations | Tech Design Template