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

Stylesheet parsing speed #357

Closed
3 tasks
yhahn opened this issue Mar 26, 2014 · 9 comments
Closed
3 tasks

Stylesheet parsing speed #357

yhahn opened this issue Mar 26, 2014 · 9 comments
Assignees

Comments

@yhahn
Copy link
Member

yhahn commented Mar 26, 2014

Please instaclose if this has been discussed/considered. As part of #356 I'd like to throw into the pot JSON-parsing speed as something to have in mind.

Some background:

  • Mapbox Streets as mapnik XML is ~500k and constitutes some 300-500ms of load time into memory on initial parsing/map object creation in mapnik,
  • I have to this point assumed that parsing JSON of an llmr style object would be trivially fast but,
  • I know from carmen work that parsing a lot of JSON and creating a lot of Array/Objects in V8 is not trivially fast,
  • @nickidlugash alerted me previously that current llmr style definitions are definitely not as detailed as carto-based styles yet -- for example, I think she mentioned like 40-50+ buckets and corresponding styles she would want

Next actions

  • Create an at-parity llmr style -- one that captures as much symbolizer/rule complexity as one of our most complex custom styles.
  • Initial benchmarks of parsing this definition to see if we should even care.
  • If we should care, benchmarks as part of style format discussion.
@edenh
Copy link
Contributor

edenh commented Mar 27, 2014

I vote for waiting on the stylesheet restructure (#356) or wherever that heads before starting this one. Trying to remake osm-bright or mapbox-streets using the current structure is very tedious especially without complex filters (and, or, not, =, !=, <, <=, >, >=, regex).

@edenh
Copy link
Contributor

edenh commented Apr 2, 2014

It seems like the restructure is going to take a while, so I went ahead recreated osm-bright:
https://github.com/mapbox/osm-bright-gl

There is probably enough to start benchmarking, but missing things are:

  • Motorway shields
  • Water wave pattern
  • POI labels: only a subset of the maki icon buckets were created
  • Text label styles (weights, wrap-width, line-spacing, etc.)

The bucket count is currently at 60. I don't see this number drastically changing even after the stylesheet restructure.

@yhahn What does the process of benchmarking look like?

@mourner
Copy link
Member

mourner commented Apr 2, 2014

@edenh please open up the repo, it's not accessible for me. I'm currently writing a migration script from current style to "the future" so this will be an excellent sample to test on. :)

@yhahn
Copy link
Member Author

yhahn commented Apr 2, 2014

@mourner should be accessible now

@yhahn
Copy link
Member Author

yhahn commented Apr 2, 2014

@edenh benchmarking JSON parsing is pretty much as simple as

  • JSON.stringify the style. We don't really care about this speed.
  • Take the resulting JSON string and JSON.parse it in a for loop like 1000x times. Time this code run with console.time/console.timeEnd or just logging a timestamp. You can then get the average time to parse by dividing the ms by number of runs.
  • You can also look into using benchmark.js for this (more statistically significant results) but a basic napkin test should give you a sense of whether the numbers are high enough for us to care : )

Beyond JSON parsing will be interesting if there are other load performance spots that a sufficiently complex style brings attention to.

@mourner
Copy link
Member

mourner commented Apr 2, 2014

@yhahn I'm sure time evaluating the JSON itself will be insignificant compared to processing the style later for rendering on each zoom frame.

@yhahn
Copy link
Member Author

yhahn commented Apr 2, 2014

Yeah, I'm assuming so too but such assumptions have burned me in the past so it's worth a quick check : )

@edenh
Copy link
Contributor

edenh commented Apr 2, 2014

Basic results:

n ms
10 0.556ms
100 0.450ms
1000 0.382ms
10000 0.346ms

@yhahn
Copy link
Member Author

yhahn commented Apr 2, 2014

\o/ time to not care

@edenh edenh closed this as completed Apr 2, 2014
bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this issue Oct 24, 2016
It involved some refactoring to keep the selection in the store,
accessible outside of simple_select. And other refactoring to
improve testability and clarify the API of the store.
bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this issue Oct 24, 2016
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

No branches or pull requests

3 participants