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

Dispose the tiles removed from the cache #2423

Merged
merged 2 commits into from Jul 28, 2014
Merged

Conversation

fredj
Copy link
Member

@fredj fredj commented Jul 22, 2014

And register the map's goog.dom.ViewportSizeMonitor to be automatically disposed.

@tschaub
Copy link
Member

tschaub commented Jul 22, 2014

Are you seeing any leaks that are addressed by this? It would be good to be able to point people to a fix when we encounter similar problems elsewhere.

In any case, this looks good to me to merge.

@fredj
Copy link
Member Author

fredj commented Jul 23, 2014

No, I don't see any leaks with the tiles because the cache (ol.TileCache) is so big (2048 entries) that the tiles are never freed.

@ahocevar
Copy link
Member

Is it really necessary to call dispose() manually? I thought the Closure library does this internally. Thanks for any clarification I could get on this.

@fredj
Copy link
Member Author

fredj commented Jul 24, 2014

It's not needed if the disposed object has not "open resources" (event listener, ...) to release.

@ahocevar
Copy link
Member

I see @fredj. So it's probably a good idea here to manually dispose(), so we don't have to remove listeners manually.

@fredj
Copy link
Member Author

fredj commented Jul 24, 2014

The tiles have no resources to release but it's probably a good practice to call dispose anyway

fredj added a commit that referenced this pull request Jul 28, 2014
Dispose the tiles removed from the cache
@fredj fredj merged commit ec8d3a4 into openlayers:master Jul 28, 2014
@fredj fredj deleted the dispose branch July 28, 2014 15:18
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

Successfully merging this pull request may close these issues.

None yet

3 participants