the right way to refresh a text layer?

6 messages Options
Embed this post
Permalink
Michal Rok

the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
Some javascript/style in this post has been disabled (why?)
Hi,
 
Somewhere in my code I have a function that is supposed to destroy and re-create a Text layer to display up-to-date information:
 
function refresh_markers() {
 map.removeLayer(markers);
 markers.destroy();
 markers = new OpenLayers.Layer.Text( "Test layer", {location: "maps/aircraft.csv?" +
    new Date().getTime() } );
 map.addLayer(markers);
}
This appears to leak memory - every time the layer is refreshed, by Explorer memory consumption grows by 5MB.
 
What is the right way to refresh a text layer?
 
 
regards,
 
Michal Rok
 

_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users
Eric Lemoine

Re: the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
On Dec 2, 2007 7:41 PM, Michal Rok <[hidden email]> wrote:

>
>
>
> Hi,
>
> Somewhere in my code I have a function that is supposed to destroy and
> re-create a Text layer to display up-to-date information:
>
> function refresh_markers() {
>  map.removeLayer(markers);
>  markers.destroy();
>  markers = new OpenLayers.Layer.Text( "Test layer", {location:
> "maps/aircraft.csv?" +
>     new Date().getTime() } );
>  map.addLayer(markers);
> }
>
> This appears to leak memory - every time the layer is refreshed, by Explorer
> memory consumption grows by 5MB.
>
> What is the right way to refresh a text layer?

Your code looks correct.

Most probably you're hitting <http://trac.openlayers.org/ticket/1123>.
You can try the patch attached to this ticket to see if that makes the
memory leak go away. If so, we should definitely move forward with
that ticket.

Thanks,

--
Eric
_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users
Michal Rok

Re: the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
> Most probably you're hitting <http://trac.openlayers.org/ticket/1123>.
> You can try the patch attached to this ticket to see if that makes the
> memory leak go away. If so, we should definitely move forward with
> that ticket.

Dear Eric,

I applied this patch but the result is the same. I suppose I'll switch to
manually manipulating each marker, and only destroying it when the marker
actually disappears from source file (the nature of my data is that 95% of
objects just move, only 5% are appear/disappear).

If you have any other patches to test out I'm happy to help.


regards,

Michal

_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users
Eric Lemoine

Re: the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
On Dec 2, 2007 9:22 PM, Michal Rok <[hidden email]> wrote:

> > Most probably you're hitting <http://trac.openlayers.org/ticket/1123>.
> > You can try the patch attached to this ticket to see if that makes the
> > memory leak go away. If so, we should definitely move forward with
> > that ticket.
>
> Dear Eric,
>
> I applied this patch but the result is the same. I suppose I'll switch to
> manually manipulating each marker, and only destroying it when the marker
> actually disappears from source file (the nature of my data is that 95% of
> objects just move, only 5% are appear/disappear).
>
> If you have any other patches to test out I'm happy to help.
Would you try the attached patch (absolutely untested!)?

--
Eric

[patch-text-layer.diff]

Index: lib/OpenLayers/Layer/Text.js
===================================================================
--- lib/OpenLayers/Layer/Text.js (revision 5325)
+++ lib/OpenLayers/Layer/Text.js (working copy)
@@ -88,7 +88,26 @@
     destroy: function() {
         this.clearFeatures();
         this.features = null;
+
+        // keep a reference on each marker, to be able to destroy
+        // each after calling Layer.Markers.destroy()
+        var markers = null;
+        if (this.markers) {
+            markers = [];
+            for (var i = 0; i < this.markers.length; i++) {
+                markers.push(this.markers[i]);
+            }
+        }
+
         OpenLayers.Layer.Markers.prototype.destroy.apply(this, arguments);
+
+        // destroy the markers
+        if (markers) {
+            for (var i = 0; i < markers.length; i++) {
+                markers[i].destroy();
+                markers[i] = null;
+            }
+        }
     },
     
     


_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users
Michal Rok

Re: the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
> Would you try the attached patch (absolutely untested!)?

This appears to destroy the same thing twice. The line:

 this.events.destroy();

in Marker.js causes a scripting error (this.events is null or not an
object) - apparently this.events does not exist at the time of execution. I
replaced this.events.destroy in Marker.js with the following:

        if (this.events != null) {
                this.events.destroy();
                this.events = null;
        }

(just like it is done with this.icon). There's no error message now, but
memory consumption keeps growing anyway.


regards,

Michal

_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users
Eric Lemoine

Re: the right way to refresh a text layer?

Reply Threaded More More options
Print post
Permalink
On Dec 5, 2007 7:19 AM, Michal Rok <[hidden email]> wrote:

> > Would you try the attached patch (absolutely untested!)?
>
> This appears to destroy the same thing twice. The line:
>
>  this.events.destroy();
>
> in Marker.js causes a scripting error (this.events is null or not an
> object) - apparently this.events does not exist at the time of execution. I
> replaced this.events.destroy in Marker.js with the following:
>
>         if (this.events != null) {
>                 this.events.destroy();
>                 this.events = null;
>         }
>
> (just like it is done with this.icon). There's no error message now, but
> memory consumption keeps growing anyway.
>
>
> regards,
>
> Michal

Thanks for testing this Michal.

I've added a new patch here <http://trac.openlayers.org/ticket/1123>.
I'm not sure at all it'll fix your problem, but it may be worth trying
it out...

Cheers,

--
Eric
_______________________________________________
Users mailing list
[hidden email]
http://openlayers.org/mailman/listinfo/users