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

Avoid unnecessary style validation #3224

Merged
merged 2 commits into from
Sep 20, 2016
Merged

Avoid unnecessary style validation #3224

merged 2 commits into from
Sep 20, 2016

Conversation

jfirebaugh
Copy link
Contributor

Avoid unnecessary style validation:

This PR does not expose a validate option at the API level yet.

Benchmarks

map-load

master c0f1c7e: 272 ms
style-load-perf b2e5679: 89 ms

style-load

master c0f1c7e: 163 ms
style-load-perf b2e5679: 112 ms

buffer

master c0f1c7e: 1,032 ms
style-load-perf b2e5679: 989 ms

fps

master c0f1c7e: 60 fps
style-load-perf b2e5679: 60 fps

frame-duration

master c0f1c7e: 7.3 ms, 0% > 16ms
style-load-perf b2e5679: 7.3 ms, 0% > 16ms

query-point

master c0f1c7e: 1.01 ms
style-load-perf b2e5679: 1.33 ms

query-box

master c0f1c7e: 64.97 ms
style-load-perf b2e5679: 68.48 ms

geojson-setdata-small

master c0f1c7e: 12 ms
style-load-perf b2e5679: 13 ms

geojson-setdata-large

master c0f1c7e: 121 ms
style-load-perf b2e5679: 113 ms

Launch Checklist

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

This code is unused following #3209.
* Don't validate mapbox:// styles
* Don't validate data in the worker
* Don't revalidate previously-validated data
@jfirebaugh jfirebaugh changed the title Validate option Avoid unnecessary style validation Sep 20, 2016
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

Looks great, 🚢

@mourner mourner merged commit 07feb8d into master Sep 20, 2016
@mourner mourner deleted the validate-option branch September 20, 2016 10:50
@mollymerp
Copy link
Contributor

daang great perf gains!! 🎉

@mourner
Copy link
Member

mourner commented Sep 20, 2016

Note that map-load benchmarks pretty much always has the branch time much lower than the master one, making this particular benchmark not very useful. Maybe it's because V8 warms up on the first run. cc @lucaswoj

@lucaswoj
Copy link
Contributor

I've noticed that too @mourner. I'll look into it.

captainbarbosa pushed a commit that referenced this pull request Oct 3, 2016
* Remove dead code

This code is unused following #3209.

* Avoid unnecessary style validation

* Don't validate mapbox:// styles
* Don't validate data in the worker
* Don't revalidate previously-validated data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants