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

Slow calls: Map.setFilter, Map.setPaintProperty, Map.setLayoutProperty #7235

Closed
gorshkov-leonid opened this issue Sep 5, 2018 · 7 comments
Closed
Labels
api 📝 performance ⚡ Speed, stability, CPU usage, memory usage, or power usage

Comments

@gorshkov-leonid
Copy link
Contributor

mapbox-gl-js version: 0.47.0

Question

Hello, we have got few critical problems with performance of Map.setFilter, Map.setPaintProperty, Map.setLayoutProperty.

  1. One root cause is that expression validation is slow. We found internal fixes Skip style validation when loading a style from mapbox.com #3152, but validate option cannot be passed via API. We applied monkey patch to disable validation. As result, view became faster (3x). Patch for Style._validate и LayerStyle._validate:
disableStyleValidation();
disableLayerStyleValidation();

function disableStyleValidation() {
    const validateDelegate = mapboxgl.Style.prototype._validate;
    mapboxgl.Style.prototype._validate = function (validate, key, value, props) {
        options = options || {};
        options.validate = false;/* <= is the salt of patch */
        return validateDelegate.apply(this, [validate, key, value, props, options]);
    };
}

function disableLayerStyleValidation() {
    const delegateConstructor = mapboxgl.Map;
    mapboxgl.Map = function() {
        delegateConstructor.apply(this, arguments);
        waitMapboxMapInitialization(this, () => disableLayerStyleInstancesValidation(this))
    };
    mapboxgl.Map.prototype = delegateConstructor.prototype;
}

function disableLayerStyleInstancesValidation(map/*: mapboxgl.Map*/) {
    for (let layerKey in map.style._layers) {
        if (!map.style._layers.hasOwnProperty(layerKey)) {
            continue;
        }
        const layer = map.style._layers[layerKey];
        if (layer._validate.fixed) {
            continue;
        }
        const validateDelegateFunction = layer._validate;
        layer.__proto__._validate = function (validatePaintProperty, key, name, value, options) {
            options = options || {};
            options.validate = false;/* <= is the salt of patch */
            return validateDelegateFunction.apply(this, [validatePaintProperty, key, name, value, options]);
        };
        layer.__proto__._validate.fixed = true;
    }
}

Do you have plans to provide the ability of validation disabling ?

Links to related documentation

  1. https://www.mapbox.com/mapbox-gl-js/api/
  2. Skip style validation when loading a style from mapbox.com #3152
@mourner mourner added performance ⚡ Speed, stability, CPU usage, memory usage, or power usage api 📝 labels Sep 5, 2018
@mourner
Copy link
Member

mourner commented Sep 5, 2018

Can you also share some examples of style values that are slow to validate? Perhaps we could improve validation performance in addition to providing a way to disable it.

@gorshkov-leonid
Copy link
Contributor Author

I'll try to isolate configuration. I need time

@gorshkov-leonid
Copy link
Contributor Author

before.zip
after.zip

I’m attaching several files with the styles used, although I’m not sure they will be helpful. Some context to understand the files:

  1. We are replacing map component in the existing system, hence there are limitations as to how the data is structured, this affects layers structure.
  2. Styles are generated programmatically, based on configurations in the existing system, so not entirely human-readable.
  3. For the case presented in attached files, there are 17 end-user layers (as defined in the existing system), which currently gets translated to 578 Mapbox layers. Some multiplication factors appear due to peculiarities in the data structure of the existing system (e.g. each end-user layer could have features of point, line, polygonal geometry). Some are due to the functionality that we need to support (e.g. there’s a notion of selection and any selected feature should be above any unselected feature- this doubles number of Mapbox layers). Some appear due to workarounds for the current limitation in mapbox-gl-js (e.g. data-driven values for line-dasharray not yet supported, hence we need 4 Mapbox layers to support 4 line patterns for each line/polygon outline layer).
  4. There’s a filtering end-user functionality, filters potentially affect features on all Mapbox layers. Think about a filter where a user could choose to show only Active or Inactive objects or both Active and Inactive- to apply this filter we need to update filters for all Mapbox layers. before.json and after.json contain styles for the map before and after application of the filter.
    I understand that number of layers is rather large and most likely we’ll be able to reduce it by several times on our side (have not optimized layers structure yet). However, we will still have substantial number of layers.
    Performance profiling shows that almost all time is spent in _validate method. Style validation is very useful during development. However, given validation performance cost and the fact that our styles are programmatically generated and tested, it would be very useful to be able to turn off style validation for production mode.

@HarelM
Copy link

HarelM commented May 13, 2019

I'm not sure if this issue is active or not but I believe it causes an issue on my end from another angle.
The following code is used to animate the direction of a line:
https://stackoverflow.com/questions/43057469/dashed-line-animations-in-mapbox-gl-js
This is related somewhat to this issue I've opened in the past: #8140
An example of a running code can be found here:
https://jsfiddle.net/2mws8y3q/

When the above tab is open in chrome the CPU starts working and never stops which causes severe performance issues on the site. It's not noticeable on strong machines but is when using old PCs or weak mobile devices.
Please let me know if you need me to open a new issue for this.
More info can be found here:
IsraelHikingMap/Site#1012
I basically would like to show the user the direction of a route, but without the animation it's hard as direction arrow heads have their drawback, which is described here:
IsraelHikingMap/Site#1030

Let me know how I can help.
Keep up the good work!

@gorshkov-leonid
Copy link
Contributor Author

gorshkov-leonid commented May 15, 2019

@HarelM I am not sure, but think it is not the same problem. I used options validate in a forked example:
https://jsfiddle.net/goleon/wdv2Lj4q and checked that style._validate did not do heavy logic. But between every two times as I can see render called 3 times. Each call of 'render' takes 10-15ms. Might be CPU is spended on these calls. Think you should create new issue.

This issue was fixed in series of releases and finished in #8189

@gorshkov-leonid
Copy link
Contributor Author

Fixed in #7604, #8189

@HarelM
Copy link

HarelM commented May 16, 2019

@gorshkov-leonid Thanks for the info and quick response! I have opened a new issue: #8253

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

No branches or pull requests

4 participants