Skip to content
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

Remove fast-stable-stringify #5154

Merged
merged 1 commit into from
Aug 16, 2017
Merged

Remove fast-stable-stringify #5154

merged 1 commit into from
Aug 16, 2017

Conversation

mourner
Copy link
Member

@mourner mourner commented Aug 16, 2017

Closes #5152 by implementing a simpler key generation for layer specifications. Input is pre-validated so we don't have to worry about circular links or weird objects, and the output doesn't have to be valid JSON, just good enough to be used as a unique key.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality covered by group_by_layout.test.js
  • post benchmark scores
  • manually test the debug page

}));
function stringify(obj) {
const type = typeof obj;
if (type === 'number' || type === 'string' || type === 'undefined')
Copy link
Contributor

@jfirebaugh jfirebaugh Aug 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also obj === null.

@mourner
Copy link
Member Author

mourner commented Aug 16, 2017

benchmark master 12aa05d remove-stable-stringify a9c76c6
map-load 228 ms 269 ms
style-load 38 ms 44 ms
buffer 628 ms 592 ms
fps 60 fps 60 fps
frame-duration 7.7 ms, 0% > 16ms 7.5 ms, 0% > 16ms
query-point 0.37 ms 0.38 ms
query-box 24.68 ms 27.03 ms
geojson-setdata-small 1 ms 1 ms
geojson-setdata-large 65 ms 65 ms

@mourner mourner merged commit 39334c7 into master Aug 16, 2017
@mourner mourner deleted the remove-stable-stringify branch August 16, 2017 20:38
willwhite pushed a commit that referenced this pull request Sep 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants