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

Performance regression for match Expressions vs. property functions (data-join technique) #6082

Closed
strech345 opened this issue Jan 30, 2018 · 13 comments
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@strech345
Copy link

For styling there is now the possebility to use Expressions. Thats nice for static data but whats about dynamic data?

For example i use vector tiles where features have only a id. At tile load i request from our server key value pairs (id -> property). Then i color it with setPaintProperty

 let dataObj = {1: 12, 2: 8, ...};
     this.map.setPaintProperty("buildings_sel", "fill-extrusion-height", ["get", ["to-string", ["get", "id"]], ["literal", dataObj]]);
    this.map.setPaintProperty("buildings", "fill-extrusion-color",
      ["match",
        ["get", ["to-string", ["get", "id"]], ["literal", dataObj]],
        1, "#4B91BD",
        2, "#B5E9FF",
       ....
        "#686868"
      ]);

For me it would be nice to also use my own function

this.map.setPaintProperty("buildings_sel", "fill-extrusion-height", festure => {
   return dataObj[feature.properties.id];
}

This could have some benefits:

  • more flexible than expressions
  • hopefully more performant
  • maybe fast to implement

I saw this on other map viewern like cesium with 3dtiles or google maps and would really like to use it in mapbox gl also.

@andrewharvey
Copy link
Collaborator

It sounds like you're asking for a way to add extra properties client side to your vector tiles. Previous discussions for this feature exist at #2671 and #4261. All the current workarounds address this by adding these extra properties into the Style rather into the data.

@strech345
Copy link
Author

Thanks @andrewharvey .
Yes. And im also searching for the right and performant way to load only nessasary data depending on tile. Most of these examples and workarounds deals with init data. I want to load data by tile load from our source and update the style. I get it working but it's not performant exspacially by using extrusion.

@strech345
Copy link
Author

One Problem i show in this video. I get one flicker when using expression on an extruded layer
https://youtu.be/QvAXKP8o_S4

@ryanbaumann
Copy link
Contributor

@strech345 try using the property function syntax when using the data join technique, instead of expression syntax. Also ensure you are using to be v0.44.

@strech345
Copy link
Author

@ryanbaumann do you mean like this example?
https://www.mapbox.com/mapbox-gl-js/example/data-join/
It's also an expression, or what do you mean?
yes im using 0.44

@ryanbaumann
Copy link
Contributor

@strech345
Copy link
Author

@ryanbaumann
big thanks. I never saw how to work with functions and for me it was not clear that i could use it in this way. Cool. For me it looks more performant than the expression.

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Feb 3, 2018

Awesome @strech345. Glad that property functions work well for your use case.

I updated this ticket to focus on improving the Expression syntax to match or exceed property function performance for large numbers of match stops, mainly for the data-join technique.

@ryanbaumann ryanbaumann changed the title Request - Function for PaintProperty to use dynamic data Performance regression for match Expressions vs. property functions (data-join technique) Feb 3, 2018
@ryanbaumann ryanbaumann added the performance ⚡ Speed, stability, CPU usage, memory usage, or power usage label Feb 3, 2018
@strech345
Copy link
Author

strech345 commented Feb 6, 2018

Hmm maybe i was wrong. i now see the glitch also with no expression or color function only by using extrusion.
https://youtu.be/BxcsnTHR9sE

This is what happend at this time.
image

The building layer only have the zoomlevel 14.

    this.map.addLayer({
      "id": "buildings",
      "type": "fill-extrusion",
      "source": "buildings",
      "source-layer": "buildings",
      "paint": {
        "fill-extrusion-color": "#686868",
        "fill-extrusion-base": 0,
        "fill-extrusion-height": ["get", "height"],
        "fill-extrusion-opacity": {
          "base": 1,
          "stops": [[13, 0], [14, 1]]
        }
      }
    });

@jfirebaugh
Copy link
Contributor

If match is significantly slower than the equivalent old style function, let's track that in a fresh ticket -- from the discussion here it's not clear to me that that's even the case.

@manassra
Copy link

manassra commented Apr 13, 2018

Hey guys, late to the discussion here, but I am also in the same boat. I have a huge data file that I need to join client side with a vector layer. Using expressions doesn't seem to be very performant. Was the performance of match indeed worse than property functions? Was there a PR addressing this? Is property functions still the recommended way to go for large joins?

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Apr 13, 2018

@manassra does switching to 0.41.0 improve the performance of your data-join expression as per #6367 ? Let's track in that ticket if so.

@manassra
Copy link

Thanks for the prompt reply, @ryanbaumann! Yes, downgrading to 0.42 seems to improve performance quite a bit indeed (especially when zooming in/out)! Will add my findings to the ticket you referenced.

Going forward, what is the more recommended way to do data joins on the client side (property functions vs expressions)? Do you guys have anything on the roadmap to make this logic easier?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⚡ Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

5 participants