Repository navigation
perf: speed up network and console store listings - #114
Merged
Merged
Conversation
Listing a network's stores ran several queries per store and loaded the whole network for distance sorting, so a page of stores could take minutes. - Store resource: build category, networks, locations and media only when the field is requested (when() evaluated its value argument for every store) - Preload the review average with withAvg() on the public and console store queries; the rating accessor uses it instead of one AVG query per store - Console store list eager-loads logo and backdrop, and networks when the network category is requested - Network category lookup reuses eager-loaded networks and resolves each network id once instead of once per store - Ignore missing or unparseable locations instead of measuring from (0, 0): no in-memory sort of every store, and maximum_distance no longer filters out every store when no location is sent - Index reviews.subject_uuid
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #114 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 2282 2295 +13
===========================================
Files 182 182
Lines 9082 9103 +21
===========================================
+ Hits 9082 9103 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Open
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.
Summary
Listing a Network's stores (
GET storefront/v1/stores) and the Console's network stores tab could take minutes. The cause was work done once per store instead of once per request, plus loading the whole network whenever distance sorting was triggered.Related Issue
Reported from device testing of the Network edition app: the stores list took close to 2 minutes.
Type of Change
Implementation Notes
Store resource:
category,networks,locationsandmedianow pass closures towhen(). PHP evaluateswhen()'s value argument even when the condition is false, so every serialized store lazy-loaded networks and ran the three-query network category lookup.Ratings: both the public network store query and the console store query preload
withAvg('reviews', 'rating').Store::getRatingAttribute()uses the preloaded average when present instead of running oneAVGquery per store. Values match the previous output: integer or float average,0when unrated.Console store list (
StoreController::onQueryRecord): eager-loadslogoandbackdrop, plusnetworkswhennetworkandwith_categoryare requested.Network category lookup: reuses eager-loaded
networks, and resolves each network id to its uuid once per process (the mapping never changes).Distance: a missing or unparseable
locationused to become (0, 0), which:maximum_distance, filtered out every store.Such requests now skip distance processing and keep SQL
limit/offset. Requests with a real location behave as before.Migration: index on
reviews.subject_uuid, guarded so it can be re-run.The other major cost was
Utils::getCountryCodeByCurrency(), which reloads the countries dataset once per store. That is fixed in fleetbase/core-api#291. Both changes are needed for the full speed-up and can be released independently.This is based on
main(v0.4.24) rather thandev-v0.4.19, which is far behind currentmain.Validation
Local note: on this macOS machine the suite exits silently partway through
Unit/Support/StorefrontTest.php, on unmodifiedmainas well. So that file was excluded locally and CI is the reference run. This change doesn't touch it.Documentation Impact
API Reference Impact
API reference notes: response shapes are unchanged. Only the no-location distance case changes:
maximum_distancewithout a usablelocationis now ignored instead of returning no stores.Risk
reviews.subject_uuidindex.ST_Distance_Sphere) would make very large networks scale further and is a follow-up.