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

add a cache of serialized style layers #8195

Closed
wants to merge 1 commit into from

Conversation

vallendm
Copy link
Contributor

@vallendm vallendm commented Apr 25, 2019

Launch Checklist

QueryRenderedFeatures has poor performance when returning lots of features. This appears to be caused by having to serialize the layer into the response. Rather than serializing the layer once - its is serialized N times if there are N features returned. This isn't normally a problem - but for a layer which has a large color expression it can take alot of time.

Rather than serialize it may times - just keep an optional cache so we only serialize each layer once.

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port

@asheemmamoowala asheemmamoowala requested review from ansis and mourner May 20, 2019 16:35
@mourner
Copy link
Member

mourner commented Jun 18, 2019

Hi @vallendm, apologies for such a long delay looking into this PR.

This appears to be a more difficult matter than I anticipated — digging into 5 year commit history around this particular feature (query results including serialized layer), I discovered that the original intent was to return evaluated layout/paint values, not original definitions — see #879. At some point during many refactorings, this changed without anyone noticing.

We could apply a temporary fix to mitigate performance issues, but an ideal solution would be to track down where that change happened, and see if we can avoid full serialization by reverting back to outputting final values of properties evaluated at the time of the query. I'll dig into this further.

@mourner
Copy link
Member

mourner commented Jun 20, 2019

I've created a new issue for this #8373 — let's track it there.

@mourner mourner closed this Jun 20, 2019
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