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

Make handling of skipped features faster #1997

Merged
merged 1 commit into from
Apr 18, 2014
Merged

Make handling of skipped features faster #1997

merged 1 commit into from
Apr 18, 2014

Conversation

elemoine
Copy link
Member

Aaron Solberg reported serious performance problems occuring when adding features to the map's skippedFeatures collection.

The issue can be reproduced on the synthetic-points example:

The issue has nothing to do with the rendering code. It's related to the way we handle "add" (and "remove") events in ol.Map when features are added to the skipped features collection. Currently, each time a feature is added we clear the skippedFeatureUids_ object and re-populate with a loop over the features in the skipped features collection. So the complexity of adding N features to a skipped features collection of length M is O(M * N).

With this PR we no longer repopulate the skippedFeatureUids_ object from scratch. The complexity of adding N features to a skipped features collection is O(N). Can be tested here: http://erilem.net/ol3/skipped-features-performance/examples/synthetic-points.html?mode=raw.

@tonio
Copy link
Member

tonio commented Apr 18, 2014

LGTM. Very noticeable effect (-:

Apart from this PR, I wonder if using skipped features is the best practice for the mailing-list use case.

@elemoine
Copy link
Member Author

Another (probably minor) optimization is to remove the "to string" conversion. I'll do that separately.

@elemoine
Copy link
Member Author

Aaron's jsfiddle changed to using my branch and skippedFeatures: http://jsfiddle.net/NC4vh/29/. Now the browser manages to complete the operation.

elemoine pushed a commit that referenced this pull request Apr 18, 2014
Make handling of skipped features faster
@elemoine elemoine merged commit 8c1c775 into openlayers:master Apr 18, 2014
@elemoine elemoine deleted the skipped-features-performance branch April 18, 2014 11:17
@tschaub
Copy link
Member

tschaub commented Apr 18, 2014

I wonder if using skipped features is the best practice for the mailing-list use case

@tonio - I wonder the same. Before exporting getSkippedFeatures let's review the use cases and make sure we're handling them in the most sensible way.

@elemoine
Copy link
Member Author

@tonio - I wonder the same. Before exporting getSkippedFeatures let's review the use cases and make sure we're handling them in the most sensible way.

Yep, same feeling here.

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