Filter keys/values based on filter results

[GretaCB]

Per @mapsam and @springmeyer 's thoughts:

In the current implementation we shave layers and whole features, but we do not (yet) drop unused properties from features that we keep. So if we keep a feature we currently decode all its properties and re-add all of its properties. If we only re-add'ed properties that are needed by a given style that would likely result in smaller, more efficient tiles for styles that only reference a few properties per style layer using vector tile sources with many properties.

vtshaver/src/shave.cpp

Line 322 in da0b5ba

* - Filter out keys/values based on filter results below. Currently we need to add them to the new layer to have a complete layer, just like name, version, extent. For each key, add it back (no filtering for now)

Starting to shave off unused properties could be a particularly good optimization for when vector tiles contain many language fields but a given style only needs to display few or none of them for a given layer.

Overall next steps to test this feature:

• Extra parsing of the style by https://github.com/mapbox/vtshaver/blob/master/lib/styleToFilters.js to collect properties we need to keep based on keys referenced in style fields like text-field and any fields referenced in expressions if the styles uses expressions (https://blog.mapbox.com/announcing-expressions-in-gl-js-a72b55d0a6af)
• Property whitelisting during shaving: Add the ability to pass down an array of extra "properties to keep" to the C++ shave function so that they are not stripped off.

[springmeyer]

Update on this: @zmofei is gearing up to start working on this. We don't have a timeline yet, but currently @zmofei is working on learning the code and how to approach this problem before fully scoping the feature.

We have early data to suggest that vector tile shaving + modern stylesheets using GL expressions can be combined for large size savings. This issue then becomes paramount in that GL expressions are explicit about what fields might be needed for expression evaluation in the client and if we can develop that whitelist we can probably further drop shaved sizes by letting go of fields not in that whitelist. So, shaving properties (this issue) + modern stylesheets using GL expressions likely == the most possible cost savings.

@zmofei will keep this ticket updated with any major progress made or blockers encountered over the next month or so.

[springmeyer]

@zmofei per chat, you've made a bunch of progress on this issue. Could you drop a note here summarizing progress so far and next steps?

[zmofei]

hey @springmeyer, sorry for reply so late (concentrate on coding and missed some tickets)

As we sync, now I finished working on the K/V filter with coding and some test.

Now we support get the properties key from:

• string format(like: roadname in {text-filed:"this this {roadname}"})
• expressions(like p2 and p4 from epressions { "exp-test2": ["==", ["has", "p2"], "false"], "exp-test4": ["feature-state", "p4"],})
• property founctions(like temperature in "circle-color": {"property": "temperature","stops": [//... ]})
  And filter the non used properties from tiles wich C++ in high performance.

The latest work is committed to key-value-filter branch.

What one thinks is that we need to discuss about this: this is the big change for vtshaver since it will auto remove the unused properties from tiles, do we need a version control to decrease the possibility of damage the used online related repos? I mean now we used the param optimize=true to open the vtshaver in our API, do we need to set another param to open the key/value filter(like optimize=level2 just example, the param need to be discussed)? In this way, we will not affect the old use of optimize=true which already published and many users have used it in they production. I think for some stable reasons we will not change any returns from our API with the old param as we upgrade our API's feature.

Next step for this feature is:

1. Discuss about the above topic.
2. Review codes with @springmeyer.
3. Make a PR(if we both think the change is ok)

[springmeyer]

Excellent work @zmofei - thanks for the update. I'll review the code later today and provide my feedback on next steps. My sense is that we should be able to test this new functionality enough in staging to build confidence in rolling out this feature in a way that would not break any existing usecases (and therefore that we don't need to version with optimize=level2. But I'll give this more thought so that, when we talk next, we can look for counter arguments or other reasons to be cautious.

[springmeyer]

@zmofei could you drop another high level update here? Please include a summary of overall progress on #20, what work remains to be done on #20 before it is ready, and what question and next steps you see. Could you do this today so that I can comment tomorrow morning my time?

[zmofei]

I draw a diagram, The grey part is what the previous version does, and the blue part is what PR#20 does, in PR#20 we added the following process:

• Extract properties from style, this includes:
  • Parsing expression sentences, Extract with properties are used.
  • Extract properties form legacy functions with property.
  • Extract properties from paint and layout string.
• Passing the properties into C++ module
• Shave properties in C++

The above was finished in #20 (comment) (Nov 22)

---

Current states:
Last week, I added a local test tool, which can create png of the tiles based on the tiles and style.json, and I found we have an issue with the shave:

I thought only paint and layout may use properties in the rendering process, the filter in style.json only to filter data have nothing to do with the render job, but after a deep test use the new tool, I realized that filter related properties also need to be used in the reader process. So the next step is to extra the properties from the filter.

To extra the properties we need to:

• Write Code to Parse and extract the used properties form deprecated Filter syntax which may be used frequently in older versions.
• Extract the Expression used properties from the filter(Which can reuse the code we already wrote)

---

After fixing this issue, I think the next step is to:

• Another deep test and code review
• Release the beta version and test
• Deploy

[zmofei]

👋 @springmeyer, deployed on the staging. And create an compare page.
https://mapbox.github.io/geo-tools/tilecompare-vtshaver/#12/31.2359/121.4968
The left side is the version before shaver, and the right side is the version after shaver. [Code]

---

Also, I made an auto compare 'tool' which can get the tiles around the given center the zoom lever 5 to 16, and auto compares 3 version:

1. Staging without shaver(which should = production without shaver)
2. Production shaver version(Only filter by filter without properties filter)
3. Starting with the latest key-value filter.

The code is in the key-value-filter-result branch performance-compare.js

Here is the result by the center [121.49677, 31.23585]

https://plot.ly/~zmofei/2.embed

In the pic, we compared the style streets-v10. The green bar on the left is the original size of the tiles, the orange bar in the middle is the old shaver, and the blue bar on the right is the new shaver, which we can find we have a large degree of optimization in our new version 🎉 🎉 .

cc w/ @chriswu42 @suntony

[springmeyer]

which we can find we have a large degree of optimization in our new version

That is an awesome result @zmofei - that indicates that shaving properties is and equal or sometimes even greater size saving than all the other previous shaving optimizations we've done (shaving off layers and features), at least for streets-v10. My hunch is that shaving properties will help even more for streets-v11 when that comes out soon /cc @nickidlugash @dasulit @tristen

The left side is the version before shaver, and the right side is the version after shaver.

Looks identical! That is amazing. Okay, I think this means we should not consider moving to "canary" deploy. For this work I'll create a new ticket to coordinate with the @mapbox/maps-api team.