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

successive calls to testLayer.addData() slow down considerably #3972

Closed
philFL opened this issue Oct 29, 2015 · 23 comments
Closed

successive calls to testLayer.addData() slow down considerably #3972

philFL opened this issue Oct 29, 2015 · 23 comments

Comments

@philFL
Copy link

philFL commented Oct 29, 2015

Hi,

I have a function that is in a timer which runs every 5 seconds.
For the sake of this test, the geojsonFeature is static.
In the real program the data will be changing.

In any event, after about the 10th time the timer runs, the time that it takes for the addData function to complete takes longer and longer. There are about 13 features in the geojsonFeatures, which are pretty much duplicates of this first entry...(I didn't show all of it here to save space)

var geojsonFeature = [{
        "type": "Feature",
        "properties": {
            "name": "Coors Field",
            "amenity": "Baseball Stadium",
            "popupContent": "This is where the Rockies play!"
        },
        "geometry": {
            "type": "Point",
            "coordinates": [-105.99404, 39.75621]
        }
    },

The time starts out at about 30ms, but pretty quickly grows to the hundreds of milliseconds. I do not understand why. I am just using the latest leaflet 0.7.7 included as follows:

    <!-- LEAFLET Libraries -->
     <link rel="stylesheet" href="http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.css" />
    <script>L_PREFER_CANVAS = true;</script>
    <script src="http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.js"></script>

The PREFER_CANAS setting doesn't seem to help, but I tried it based on other posts.

Sample Code:

    var time1;
    var time2;
    if (testLayer != null) {
        testLayer.clearLayers();
        time1 = Date.now();
        testLayer.addData(geojsonFeature);
        time2 = Date.now();
    }
    else {
        time1 = Date.now();
        testLayer = L.geoJson(geojsonFeature).addTo(map);
        time2 = Date.now();
    }
    var msg = '<br/>test' + ' time add to map: ' + (time2 - time1);
    $('#info span').append(msg);

Perhaps I am missing something obvious. Any help will be greatly appreciated.

@IvanSanchez
Copy link
Member

but pretty quickly grows to the hundreds of milliseconds. I do not understand why.

Uuummm, profile your javascript code? This doesn't sound like a bug in Leaflet so far.

@philFL
Copy link
Author

philFL commented Oct 29, 2015

I will try that. Thank you. However the code is not doing much besides clearing the layer and then putting the data back in the layer. I have the time stamp immediately before and after this function call and I have no control over what happens in that function.

Thank you

Sent from my iPhone

On Oct 28, 2015, at 9:29 PM, Iván Sánchez Ortega notifications@github.com wrote:

but pretty quickly grows to the hundreds of milliseconds. I do not understand why.

Uuummm, profile your javascript code? This doesn't sound like a bug in Leaflet so far.


Reply to this email directly or view it on GitHub.

@IvanSanchez
Copy link
Member

Don't use timestamps. Use the debugging capabilities for your browser to profile nested calls, and use performance.now() instead of Date.now(). If there's an obvious performance hog in the core leaflet code, it might be worth reporting, but keep in mind the 0.7.x code will not probably be improved since all the work is focused on 1.0.0.

See https://developer.mozilla.org/en-US/docs/Tools/Performance/Call_Tree and https://developer.mozilla.org/en-US/docs/Web/API/Performance/now

And please try to use the github issue tracker only for bugs in the future.

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Ok. Thank you. I am new to this process. I tried searching the web for an answer before posting this issue. I apologize for any inconvenience I caused.
Phil

Sent from my iPhone

On Oct 29, 2015, at 4:06 AM, Iván Sánchez Ortega notifications@github.com wrote:

Don't use timestamps. Use the debugging capabilities for your browser to profile nested calls, and use performance.now() instead of Date.now(). If there's an obvious performance hog in the core leaflet code, it might be worth reporting, but keep in mind the 0.7.x code will not probably be improved since all the work is focused on 1.0.0.

See https://developer.mozilla.org/en-US/docs/Tools/Performance/Call_Tree and https://developer.mozilla.org/en-US/docs/Web/API/Performance/now

And please try to use the github issue tracker only for bugs in the future.


Reply to this email directly or view it on GitHub.

@IvanSanchez
Copy link
Member

Don't worry much, it's just a bit annoying. We've been discussing ways of making users ask questions in better places (see #3886), but we have no perfect solution yet.

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Ivan,

I am sorry to bother you one more time, but I ran the profiler and got the following result. This addDoubleTapListener is what takes longer and long to complete. I don’t think we need to support any double tapping in this application. Is there a way to prevent this from being setup?

Thank you for any guidance you can provide.

Phil

From: Iván Sánchez Ortega [mailto:notifications@github.com]
Sent: Thursday, October 29, 2015 6:30 AM
To: Leaflet/Leaflet Leaflet@noreply.github.com
Cc: philFL sander@go-intech.com
Subject: Re: [Leaflet] successive calls to testLayer.addData() slow down considerably (#3972)

Don't worry much, it's just a bit annoying. We've been discussing ways of making users ask questions in better places (see #3886 #3886 ), but we have no perfect solution yet.


Reply to this email directly or view it on GitHub #3972 (comment) . https://github.com/notifications/beacon/AOqNVQ4bzc71WRUPgC2kCS8amQ4M1EYiks5pAeyvgaJpZM4GX3QZ.gif

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Ivan,

I think that perhaps this is what I am experiencing.

#2820

Phil

From: Iván Sánchez Ortega [mailto:notifications@github.com]
Sent: Thursday, October 29, 2015 4:06 AM
To: Leaflet/Leaflet Leaflet@noreply.github.com
Cc: philFL sander@go-intech.com
Subject: Re: [Leaflet] successive calls to testLayer.addData() slow down considerably (#3972)

Closed #3972 #3972 .


Reply to this email directly or view it on GitHub #3972 (comment) . https://github.com/notifications/beacon/AOqNVegDpZNw1rapVGxf2GgVLJxmi7GOks5pAcsFgaJpZM4GX3QZ.gif

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Ivan,

After running the profiler as you suggested, I found that the issue was related to addDoubleTapListener.

I downloaded the 0.7.7 and commented out any code related to the use of dblClick. This cleared up my problem.

Thank you for your guidance regarding the profiler. I used the one available in IE11 to get the listing shown above.

I apologize if this is not the correct place to respond, but I thought it might be useful information.

Phil

@mourner mourner reopened this Oct 29, 2015
@mourner
Copy link
Member

mourner commented Oct 29, 2015

@danzel please take a look.

@danzel
Copy link
Member

danzel commented Oct 29, 2015

ok!

@danzel
Copy link
Member

danzel commented Oct 29, 2015

I can't reproduce this locally in Chrome.

What browser are you using? Do you have a touch device?
Can you get me a call stack from addDoubleTapListener when it gets slow?

I left this running for 2 minutes (at 10 updates a second) and an update took the same amount of time at the end as it did at the start. (24ms)

here is my code:

<!DOCTYPE html>
<html>
<head>
    <title>Leaflet debug page</title>

    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

    <link rel="stylesheet" href="../../dist/leaflet.css" />

    <link rel="stylesheet" href="../css/mobile.css" />

    <script type="text/javascript" src="../../build/deps.js"></script>
    <script src="../leaflet-include.js"></script>
</head>
<body>

    <div id="map"></div>

    <script type="text/javascript">

        var osmUrl = 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',
            osmAttrib = '&copy; <a href="http://openstreetmap.org/copyright">OpenStreetMap</a> contributors',
            osm = L.tileLayer(osmUrl, {maxZoom: 18, attribution: osmAttrib}),
            latlng = new L.LatLng(39.75621, -105.99404);

        var map = new L.Map('map', {center: latlng, zoom: 15, layers: [osm]});

        var geojsonFeature = [{
            "type": "Feature",
            "properties": {
                "name": "Coors Field",
                "amenity": "Baseball Stadium",
                "popupContent": "This is where the Rockies play!"
            },
            "geometry": {
                "type": "Point",
                "coordinates": [-105.99404, 39.75621]
            }
        }];

        var testLayer;

        setInterval(function() {
            var time1;
            var time2;
            if (testLayer != null) {
                testLayer.clearLayers();
                time1 = performance.now();
                testLayer.addData(geojsonFeature);
                time2 = performance.now();
            }
            else {
                time1 = performance.now();
                testLayer = L.geoJson(geojsonFeature).addTo(map);
                time2 = performance.now();
            }
            //console.log((time2 - time1));
        }, 100);
    </script>
</body>
</html>

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Hi

I am running IE11 as that is what our customer uses.

No touch device.

I can get you a call stack later this evening or first thing in the morning. It is 6pm here

Phil

Sent from my iPhone

On Oct 29, 2015, at 5:33 PM, Dave Leaver notifications@github.com wrote:

I can't reproduce this locally in Chrome.

What browser are you using? Do you have a touch device?
Can you get me a call stack from addDoubleTapListener when it gets slow?

I left this running for 2 minutes (at 10 updates a second) and an update took the same amount of time at the end as it did at the start. (24ms)

here is my code:

<title>Leaflet debug page</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

<link rel="stylesheet" href="../../dist/leaflet.css" />

<link rel="stylesheet" href="../css/mobile.css" />

<script type="text/javascript" src="../../build/deps.js"></script>
<script src="../leaflet-include.js"></script>
<div id="map"></div>

<script type="text/javascript">

    var osmUrl = 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',
        osmAttrib = '&copy; <a href="http://openstreetmap.org/copyright">OpenStreetMap</a> contributors',
        osm = L.tileLayer(osmUrl, {maxZoom: 18, attribution: osmAttrib}),
        latlng = new L.LatLng(39.75621, -105.99404);

    var map = new L.Map('map', {center: latlng, zoom: 15, layers: [osm]});

    var geojsonFeature = [{
        "type": "Feature",
        "properties": {
            "name": "Coors Field",
            "amenity": "Baseball Stadium",
            "popupContent": "This is where the Rockies play!"
        },
        "geometry": {
            "type": "Point",
            "coordinates": [-105.99404, 39.75621]
        }
    }];

    var testLayer;

    setInterval(function() {
        var time1;
        var time2;
        if (testLayer != null) {
            testLayer.clearLayers();
            time1 = performance.now();
            testLayer.addData(geojsonFeature);
            time2 = performance.now();
        }
        else {
            time1 = performance.now();
            testLayer = L.geoJson(geojsonFeature).addTo(map);
            time2 = performance.now();
        }
        //console.log((time2 - time1));
    }, 100);
</script>
— Reply to this email directly or view it on GitHub.

@philFL
Copy link
Author

philFL commented Oct 29, 2015

Hi,

Thank you for looking into this.

When I tried to run the code you supplied, I received this error

Unhandled exception at line 27, column 13 in http://localhost/PATMapGIU_Prototype/Default.aspx

0x800a1391 - JavaScript runtime error: 'L' is undefined

So I changed out the links to leaflet to what you see below (and which I see on the leafletjs.com site)…which is what I was using when I saw the issue.

I used your code and it did not seem to show the problem. Console showed ~2 to 3 ms.

Then, I noticed you on had only one feature item.

I added a few more and ran it and the problem shows up right away (that is, by the time I get into IE and go to the tools menu and select the F12 Developer tools view and select the Console tab…..it is up to ~300ms and growing).

Please let me know if I can provide anything else.

Phil

PS While it didn’t seem to affect anything, I didn’t get the map showing up. Just a blank page. The console showed the logging fine, though, so I didn’t try to debug that.

<title>Leaflet debug page</title>



<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

<%--

<link rel="stylesheet" href="../css/mobile.css" />



<script type="text/javascript" src="../../build/deps.js"></script>

<script src="../leaflet-include.js"></script>--%>

<link rel="stylesheet" href="http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.css" />

<script src="http://cdn.leafletjs.com/leaflet/v0.7.7/leaflet.js"></script>



 </head>
<div id="map"></div>

<div id="info"><span>Debug information</span></div>



<script type="text/javascript">



    var osmUrl = 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',

        osmAttrib = '&copy; <a href="http://openstreetmap.org/copyright">OpenStreetMap</a> contributors',

        osm = L.tileLayer(osmUrl, {maxZoom: 18, attribution: osmAttrib}),

        latlng = new L.LatLng(39.75621, -105.99404);



    var map = new L.Map('map', {center: latlng, zoom: 15, layers: [osm]});



var geojsonFeature = [{

    "type": "Feature",

    "properties": {

        "name": "Coors Field",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-105.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fielda",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-106.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldb",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-107.99404, 39.75621]

   }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldc",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-108.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldd",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-109.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fielde",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-110.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldf",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-111.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldaa",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-106.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldbb",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-107.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldcc",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-108.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fielddd",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

       "coordinates": [-109.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldee",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-110.99404, 39.75621]

    }

}, {

    "type": "Feature",

    "properties": {

        "name": "Coors Fieldff",

        "amenity": "Baseball Stadium",

        "popupContent": "This is where the Rockies play!"

    },

    "geometry": {

        "type": "Point",

        "coordinates": [-111.99404, 39.75621]

    }

}];



    var testLayer;



    setInterval(function() {

        var time1;

        var time2;

        if (testLayer != null) {

            testLayer.clearLayers();

            time1 = performance.now();

            testLayer.addData(geojsonFeature);

            time2 = performance.now();

        }

        else {

            time1 = performance.now();

            testLayer = L.geoJson(geojsonFeature).addTo(map);

            time2 = performance.now();

        }

        console.log((time2 - time1));

    }, 100);

</script>

From: Dave Leaver [mailto:notifications@github.com]
Sent: Thursday, October 29, 2015 5:33 PM
To: Leaflet/Leaflet Leaflet@noreply.github.com
Cc: philFL sander@go-intech.com
Subject: Re: [Leaflet] successive calls to testLayer.addData() slow down considerably (#3972)

I can't reproduce this locally in Chrome.

What browser are you using? Do you have a touch device?
Can you get me a call stack from addDoubleTapListener when it gets slow?

I left this running for 2 minutes (at 10 updates a second) and an update took the same amount of time at the end as it did at the start. (24ms)

here is my code:

<title>Leaflet debug page</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

<link rel="stylesheet" href="../../dist/leaflet.css" />

<link rel="stylesheet" href="../css/mobile.css" />

<script type="text/javascript" src="../../build/deps.js"></script>
<script src="../leaflet-include.js"></script>
<div id="map"></div>

<script type="text/javascript">

    var osmUrl = 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png',
        osmAttrib = '&copy; <a href="http://openstreetmap.org/copyright">OpenStreetMap</a> contributors',
        osm = L.tileLayer(osmUrl, {maxZoom: 18, attribution: osmAttrib}),
        latlng = new L.LatLng(39.75621, -105.99404);

    var map = new L.Map('map', {center: latlng, zoom: 15, layers: [osm]});

    var geojsonFeature = [{
        "type": "Feature",
        "properties": {
            "name": "Coors Field",
            "amenity": "Baseball Stadium",
            "popupContent": "This is where the Rockies play!"
        },
        "geometry": {
            "type": "Point",
            "coordinates": [-105.99404, 39.75621]
        }
    }];

    var testLayer;

    setInterval(function() {
        var time1;
        var time2;
        if (testLayer != null) {
            testLayer.clearLayers();
            time1 = performance.now();
            testLayer.addData(geojsonFeature);
            time2 = performance.now();
        }
        else {
            time1 = performance.now();
            testLayer = L.geoJson(geojsonFeature).addTo(map);
            time2 = performance.now();
        }
        //console.log((time2 - time1));
    }, 100);
</script>


Reply to this email directly or view it on GitHub #3972 (comment) . https://github.com/notifications/beacon/AOqNVX9urQig8ytJ0QCzMmE2CrbYy8A2ks5pAogpgaJpZM4GX3QZ.gif

@danzel
Copy link
Member

danzel commented Oct 30, 2015

It reproduces in IE11, left it running over lunch and now it takes a second to add and remove the layer, will check it out.

@danzel
Copy link
Member

danzel commented Oct 30, 2015

Here is the call stack.
image

@danzel
Copy link
Member

danzel commented Oct 30, 2015

hahaha, ok got it.

When a marker is added to the map we add a double tap listener.
When it is removed, we don't remove the event listener, we normally don't need to. But in the pointer implementation we have a global event listener, and this never gets removed, so we end up adding heaps of them.
https://github.com/Leaflet/Leaflet/blob/stable/src/dom/DomEvent.DoubleTap.js#L84
https://github.com/Leaflet/Leaflet/blob/stable/src/dom/DomEvent.DoubleTap.js#L98-L99

This was fixed in master in e41c131 by #3007

Probably not worth backporting to stable.
Only affects repeatedly adding and removing markers on IE(11/10) with a touch device.

@mourner over to you :)

@danzel
Copy link
Member

danzel commented Oct 30, 2015

@philFL this is fixed on master, grab one of the 1.0.0 beta builds.

@philFL
Copy link
Author

philFL commented Oct 30, 2015

Thank you, Dave.
Sorry I didn't sent the call stack earlier, I thought I had posted it then got side tracked with the code sample you sent.

Phil

Sent from my iPhone

On Oct 29, 2015, at 9:18 PM, Dave Leaver notifications@github.com wrote:

@philFL this is fixed on master, grab one of the 1.0.0 beta builds.


Reply to this email directly or view it on GitHub.

@markstuart
Copy link

I'm also seeing significant slowdown on IE11/10 when calling addData on a GeoJSON layer when passing a FeatureCollection with ~2000 features. The browser hangs and displays a 'long running script' message after a period of inactivity. I see the implementation of GeoJSON layer calls this.addData for each feature in the collection, do you think this would be related @danzel?

See https://github.com/Leaflet/Leaflet/blob/master/src/layer/GeoJSON.js#L26 for context.

@danzel
Copy link
Member

danzel commented Nov 9, 2015

Are you using a 1.0.0 beta?

@markstuart
Copy link

I am now, and it fixes the issue. I was mostly trying to expand on the possible use cases that would show up this issue. My concern is that the current stable version of leaflet is non-performant under IE with what I would consider normal usage. Is this worth a v0.7.8?

@philFL
Copy link
Author

philFL commented Nov 15, 2015

Ivan,

I hate to bother you, but I am not sure of the correct place to ask about this issue where CircleMarker labels show up on top of the pop up. I found someone else posted a question about this here http://gis.stackexchange.com/questions/133642/leaflet-removing-labels-when-any-popup-window-is-shown , but no answer was posted.

I noticed that issue 3908 addressed bringing popups to the front, but I can’t switch to the beta at the moment, so I don’t know if that fixes this issue. I tried snagging the code used for that fix and put it in the version of Leaflet I have 0.7.7. It executes, but the screen still looks as shown.

I already searched the web for an answer but didn’t see one, otherwise I would not bother you.

Thanks for any guidance,

Phil

From: Iván Sánchez Ortega [mailto:notifications@github.com]
Sent: Thursday, October 29, 2015 6:30 AM
To: Leaflet/Leaflet Leaflet@noreply.github.com
Cc: philFL sander@go-intech.com
Subject: Re: [Leaflet] successive calls to testLayer.addData() slow down considerably (#3972)

Don't worry much, it's just a bit annoying. We've been discussing ways of making users ask questions in better places (see #3886 #3886 ), but we have no perfect solution yet.


Reply to this email directly or view it on GitHub #3972 (comment) . https://github.com/notifications/beacon/AOqNVQ4bzc71WRUPgC2kCS8amQ4M1EYiks5pAeyvgaJpZM4GX3QZ.gif

@IvanSanchez
Copy link
Member

I'm gonna close this as a duplicate of #4357 - issue is pretty much the same (lots of stuff in IE and doubleTap event handler)

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

No branches or pull requests

5 participants