[Performance] Replace O(n²) array scans in appDiff with Map/Set lookups - #8599
Draft
github-actions[bot] wants to merge 1 commit into
Draft
github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
appDiff runs on every app reload during `shopify app dev`, so it executes on each file change. It scanned the opposite extension array once per extension, making it O(n²) in the number of extensions. Index the old extensions by uid in a Map and the new uids in a Set so created/deleted/updated detection is O(n) with O(1) lookups. Behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
WHY are these changes introduced?
appDiffcompares the extensions of the previously loaded app against a freshly reloaded one. It runs insideReloadAppHandler, so it executes on every app reload duringshopify app dev— that is, on every relevant file change while the dev server is running.The implementation scanned the opposite extension array once per extension:
newExtensions.filter((ext) => !oldExtensionsUids.includes(ext.uid))oldExtensions.filter((ext) => !newExtensionsUids.includes(ext.uid))oldExtensions.find((oldExt) => oldExt.uid === ext.uid)inside afilterovernewExtensionsEach of those is a linear scan nested inside a loop, making the whole function O(n²) in the number of extensions. Apps with many extensions pay this cost on each keystroke-triggered reload, right on the dev feedback loop.
WHAT is this pull request doing?
Index the old extensions by
uidin aMapand the new uids in aSet, so every membership test and lookup is O(1) instead of a full array scan. The diff is now O(n) overall.The
Mapdoubles as both the membership check for created extensions and the lookup for the updated comparison, so only two index structures are built instead of the twouidarrays that existed before — no extra allocation.Behaviour is unchanged: the same extensions are reported as created, deleted, and updated, in the same order.
uidwas already used as the identity key, so aMap/Setkeyed on it preserves the original semantics exactly.Expected impact: reload-time diffing drops from quadratic to linear. Negligible for a handful of extensions, but it removes a scaling cliff on the hottest path of
app devfor apps with many extensions.How to manually test your changes?
Then edit an extension's
.toml(to trigger an update), add a new extension folder, and delete one. Confirm the dev server reports the created, updated, and deleted extensions exactly as before.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add