Performance issue with L.Util.formatNum in Leaflet 1.5

[doubliez]

• I've looked at the documentation to make sure the behavior is documented and expected
• I'm sure this is a Leaflet code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
• I've searched through the issues to make sure it's not yet reported

Steps to reproduce

• Map with a few hundred GeoJSON polygons drawn at the scale of a city
• onEachFeature I add mouseover and mouseout listeners which call layer.setStyle in order to highlight the polygons as the user moves their mouse over them

In summary I do something like:

const onEachFeature = (feature, layer) => {
    layer.on('mouseover', () => {
        layer.setStyle({ fillOpacity: 0.2 });
    });
    layer.on('mouseout', () => {
        layer.setStyle({ fillOpacity: 0 });
    });
};

const overlay = new L.GeoJSON(null, {
    onEachFeature
});

overlay.addTo(map);

for (const feature of features) {
    overlay.addData(feature);
}

Expected behavior
As the user moves the mouse over the polygons, it smoothly highlights them (by changing fillOpacity) without any lag.

This is the behavior in Leaflet 1.4.

Current behavior
As of Leaflet 1.5, the transition between each polygon is very laggy and user experience is degraded.

Profiling the performance in Chrome, I noticed the culprit is the formatNum function.

The mouseover and mouseout events used to take about 25ms, now they take about 120ms, and it's clearly noticeable.

Looking at the diff between Leaflet 1.4 and 1.5, I noticed the formatNum function indeed has been changed. The related PR is #6587

A small benchmark http://jsben.ch/D3VkH shows the performance difference. I suppose the number of times formatNum is called is proportional to the number of polygons in my GeoJSON layer, because the performance impact becomes noticeable when more than about 100 polygons are drawn on the map.

Environment

• Leaflet version: 1.5.1
• Browser (with version): Chrome Version 74.0.3729.157 (Official Build) (64-bit)
• OS/Platform (with version): macOS Mojave 10.14.4

Additional context

Minimal example reproducing the issue

I don't have time to produce a minimal example right now, but I can confirm that if I change the code of the formatNum function in Leaflet 1.5 to what it was in 1.4, the issue is resolved.

[IvanSanchez]

cc @inkstak

I'm looking at https://leafletjs.com/examples/choropleth/example.html (which is using 1.5.1 as of today), and I can see no noticeable slowdown nor lagginess.

Can you publish a fiddle/codepen/plunkr that displays this behaviour? (the lagginess, I mean)

While I agree that formatNum is slower, it seems to be 15% slower, not something worrisome like 500% slower. Unless I can profile this myself, I must question whether formatNum is the root cause or not.

[cherniavskii]

@IvanSanchez it's actually ~99 times slower, according to https://jsperf.com/leaflet-formatnum-performance/1 (for some reason JSperf says it's 99% slower, but look at ops/sec scores)

[cherniavskii]

Which makes me willing to revert #6587 for the sake of performance

[inkstak]

Hi, sorry for the performance issue.

Could we introduce an option to choose the method to round numbers ?
I guess the 1.4 way may be the default one : performances over accuracy, but being able to opt for accuracy must be possible.

[cherniavskii]

Hi @inkstak

We try to keep Leaflet as simple as possible.
Keeping second formatNum function means, that we have to introduce additional configuration in 3 other methods (in 3 different classes).

Given that this level of precision is not necessary for vast majority of Leaflet users, it would be an overkill to support this functionality in Leaflet core.

That being said, you're still able to customize Leaflet to suit your needs.
You'll need to override GeoJSON.latLngToCoords method, which is very easy - see https://leafletjs.com/examples/extending/extending-1-classes.html

You can even go a step further and publish it a standalone plugin (which can be added to our plugins page).

And lastly, thanks for taking your time contributing to Leaflet, it's always greatly appreciated!

[IvanSanchez]

It seems that I misread the jsperf reports here. I must agree with @cherniavskii.

[inkstak]

Just a quick feedback : overwriting is not so easy, due to how Leaflet uses internal API.

Almost every toGeoJSON functions use the internal declaration of latLngToCoords.
I'll have to duplicate each one of them just to use my overwritten function latLngToCoords.

Doing the job is not a problem, but maintaining the patch (of the plugin) will be more complicated.

[desean1625]

This commit breaks formatting of numbers that are in scientific notation. This causes errors when trying to use togeojson on countries boarders or features that are near the equator. Causing malformed geojson.

var line = L.polyline([[0,-7.993322e-10],[10,20],[20,30]]);
L.geoJson(line.toGeoJSON());//errors on NaN
L.Util.formatNum(-7.993322e-10); //Returns NaN

[johnd0e]

The issue is really weird.
How have it ever happened that formatNum is included in hot code path?

In sources I see that it is used only when encoding leaflet layers into GeoJSON objects.
So it should never affect mouseover handler.

@doubliez ?

Can you publish a fiddle/codepen/plunkr that displays this behaviour? (the lagginess, I mean)

I've tried to do this with 1000 polygons: https://jsbin.com/kusigur/edit?js,output
And I do not observe any performance issues.

[doubliez]

@johnd0e Leaflet 1.6 doesn't suffer from this performance issue anymore. I haven't looked at what changed, I just upgraded to 1.6 and my application works fine (unlike with 1.5).

[johnd0e]

I know.
Still I would like to find out why that ever could happen.
So I will appreciate if you provide some code sample.

[doubliez]

@johnd0e Recently I looked into performance issues in our application and found what could have been the cause and that I didn't realize earlier.

In my initial post I left out something important: in both the mouseover and mouseout handler, I'm also doing a setState (it's a React app) to show some information to the user about the polygon being hovered. During profiling I saw that a lot of time was spent on updating the state.

Now it doesn't change the fact that there was a performance degradation in Leaflet 1.5 that was solved in 1.6, but it was probably exacerbated by the fact that I'm calling setState which was already slowing down my mouseover and mouseout handlers.

Sorry but I don't really have time to produce a code sample, and without React involved in the mix I probably wouldn't be able to reproduce any performance issue.

[johnd0e]

@doubliez
Ok, thanks.