-
-
Notifications
You must be signed in to change notification settings - Fork 717
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lazy serialization of layers #2306
Merged
Merged
Conversation
This file contains 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
* Warnonce for Terrain Source (maplibre#2298) * check for terrain source with hillshade and issue warnonce * changelog update * add test and adjust indentation * improve changelog note * move test to bottom * test updated to check console calls * syntax for CI pass --------- Co-authored-by: Harel M <[email protected]> * fix lint * add test * change log --------- Co-authored-by: Victor Gerard Temprano <[email protected]> Co-authored-by: Harel M <[email protected]>
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
HarelM
reviewed
Mar 28, 2023
Thanks for taking the time to open this PR! |
HarelM
reviewed
Mar 29, 2023
HarelM
reviewed
Mar 29, 2023
HarelM
approved these changes
Mar 29, 2023
8 tasks
This was referenced Jul 30, 2024
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.
Launch Checklist
this._serializedLayers dictionary is only needed when the style itself is serialized OR queryRenderedFeatures is called.
Currently it is populated upon creation, which is an overhead for initial map load. This PR changes to lazy serialization.
Based on our experiment, for 150 layers this change saves ~20ms at 75 percentile, and ~100ms at 90 percentile.
Additional changes:
"_serializeLayers" is renamed to "_serializeByIds" and piggyback on this._serializedLayers dictionary for better performance. Previously it was doing fresh serialization each time.
"setState" method is serializing current instance twice. Fixed.
replace forEach with simple for/of loop in "setState". Better performance, easier to debug
creating layers is the most time-consuming part of _load method (~80% of time), so split out to be its own method for easier profiling in Chrome performance tool.
Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
Briefly describe the changes in this PR.
Write tests for all new functionality.
Add an entry to
CHANGELOG.md
under the## main
section.