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

blocking initialization (for large data sets) #292

Closed
mindplay-dk opened this issue Dec 19, 2013 · 16 comments

Comments

@mindplay-dk
Copy link
Contributor

commented Dec 19, 2013

The README says:

Performance optimizations could be done so these are handled more gracefully (Running the initial clustering over multiple JS calls rather than locking the browser for a long time)

Any plans to do that?

I have a set of 8,000 points, and I followed the provided 50K example - this plug-in is amazing and works wonders for making large data sets intelligible to the end user - but currently I can't deploy it in production because it triggers the "slow page" dialog in IE8, and in newer browsers on slower machines.

I want to help, and I did some profiling and narrowed it down the to MarkerClusterGroup._addLayer which seems to be the bottleneck.

Optimizations, sure, but that won't help much in practice, since initialization on slower machines will still trigger the slow page dialog - the large initialization operation needs to be broken up and give time back to the browser/OS, and perhaps (preferably) provide some sort of callback that enables a developer to display a more friendly/relevant message to users on slow systems.

I haven't been able to pinpoint a loop in the MarkerCluster plug-in itself that I could break up into smaller tasks in this manner...

Any idea where to start?

@danzel

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

I'll prototype this up.
Any idea how many points before the slow js dialog comes up?

@danzel

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Almost got this working. Found a performance issue too.
addLayers in the case where we are not on the map is really slow, we add every marker to _needsClustering one by one, each time checking if the marker is already in there.
This means we check every marker we are adding against every other marker we are adding, which is a waste of time.

danzel added a commit that referenced this issue Dec 19, 2013

@danzel

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Okay, managed to improve performance while doing this, so a win over all :)
The 50000 example went from 6 seconds to 3, even when using chunked loading!

https://github.com/Leaflet/Leaflet.markercluster/tree/better-loading

New options:
chunkedLoading - bool, default false. Set to true for chunked loading.
chunkSize - default 500. How many markers to add in a chunk.

Please give this a test, we probably want to add some events for chunk loading begin/end.
The unit tests seem to break for me too, they just lock up part way through. My build environment is a total mess atm however, so will look at this later.

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 19, 2013

Tested, and loads without error on a slow system with IE8! Fantastic work, thank you! :-)

The system felt very bogged down though, so I experimented with fine-tuning some of the values - in addLayers() I give back 20 msec to the system, e.g. 1 frame at 50 Hz, so setTimeout(process, 20);, and with a much smaller chunk size (50) the map becomes responsive and can be panned/zoomed while the data is still loading in the background.

The numbers are going to be system-dependent though - perhaps it would work better not to process fixed-size chunks? Instead, use Date to instrument how long we've been blocking - for example, check elapsed time every 50 markers, and give back 5 msec to the system after processing for 50 milliseconds.

I could fork and attempt this myself, if you'd like me to?

we probably want to add some events for chunk loading begin/end.

I could also fork and implement this myself, if you'd like? You've already done the difficult bit :-)

@danzel

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Sure that'd be awesome :)

Depending on the users use case, they may have a loading screen showing, so they could live with the system being less responsive if it meant loading was faster.

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 23, 2013

I'm ignoring this for now and concentrating on some of the other issues we discussed above - but I noticed that, if you drag the map a little while a large bulk of markers is still loading/initializing, the map will start incrementally painting itself before it's done loading... I'm seeing the actual numbers on the map counting up, while the clusters are being created.

If this is hard to avoid, it's probably not worth spending too much time on - my only concern was that the browser may be doing extra work attempting to paint between each chunk, and that some unnecessary updates may be happening - including the work of updating the actual markers (with counts and colors) before they're actually painted for the first time.

If you refresh the page and leave it alone while it's initializing, it doesn't paint - so maybe dragging the map slightly is what triggers the first and subsequent redraws.

Also note that dragging the map is only possible in the first place with e.g. setTimeout(process, 20) as far as I can tell.

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 23, 2013

Also just noticied in the 50K marker example, if I add console.trace('adding', layersArray.length, 'markers'); at the beginning of MarkerClusterGroup.addLayers, I see the following on the console:

adding 50000 markers MarkerClusterGroup.js:161
L.MarkerClusterGroup.L.FeatureGroup.extend.addLayers MarkerClusterGroup.js:161
(anonymous function) marker-clustering-realworld.50000.html:61

adding 50000 markers MarkerClusterGroup.js:161
L.MarkerClusterGroup.L.FeatureGroup.extend.addLayers MarkerClusterGroup.js:161
L.MarkerClusterGroup.L.FeatureGroup.extend.onAdd MarkerClusterGroup.js:463
o.Map.o.Class.extend._layerAdd leaflet.js:6
o.Map.o.Class.extend.addLayer leaflet.js:6
(anonymous function)

Looks like addMarkers() is being called twice - is that supposed to happen?

Putting console.log('- processing', start, 'to', end); inside process() does confirm that the actual processing only occurs once, so maybe this is normal.

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 24, 2013

Another (unrelated?) issue with the 50K example: move the mouse cursor away from the Leaflet control, then refresh the page and leave the mouse sitting still... the markers never appear - until you move the mouse cursor over Leaflet, then they show up.

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 24, 2013

Submitted a pull request and not sure what I did wrong, but it wasn't referenced here.

Merry Christmas, or whatever you happen to celebrate this time of year :-)

@danzel

This comment has been minimized.

Copy link
Member

commented Dec 24, 2013

Awesome, will take a look through in the next few days. Merry Christmas to you too :)

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Dec 25, 2013

One thought: I'm still sometimes seeing progressive updates while it's loading, and expect this would occur if the make was active and in use before the loading of a large data set is triggered.

Is there any way (and would it make sense) to disable visual updates of the layer group being populated, or of Leaflet altogether (probably less desirable) - while the bulk loading is taking place?

@danzel

This comment has been minimized.

Copy link
Member

commented Jan 9, 2014

If you start doing things to the MCG while it is still loading then you might see some markers appear on the map. Other bad things could happen too.
We might need to not listen to map movement events while we are chunk loading, as we'll add partial markers to the map as you found. Or be simpler and just have client apps be told not to allow map interaction while we are chunk loading.

addMarkers gets called twice, it does different things both times (one adding markers into an array, one actually doing things)

Not sure what is up with nothing drawing if the mouse isn't on, sounds weird!

@mindplay-dk

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2014

Not sure what is up with nothing drawing if the mouse isn't on, sounds weird!

I think this was due to Chrome Aura which sneaked into a bunch of people's machines via the stable channel - I was one of the lucky contenders.

@danzel

This comment has been minimized.

Copy link
Member

commented Jan 10, 2014

lol :(

danzel added a commit that referenced this issue Jan 20, 2014

@danzel

This comment has been minimized.

Copy link
Member

commented Jan 20, 2014

Cool, all merged and such, should be good :)

@danzel danzel closed this Jan 20, 2014

@danzel

This comment has been minimized.

Copy link
Member

commented Jan 21, 2014

Woops, thought I'd merged to master. I have now :)

JamesRezo pushed a commit to spipremix/gis that referenced this issue Jun 10, 2018

brunobergot@gmail.com
Version 4.17.0 : début de refonte de la page exec=gis_tous + maj des …
…libs

- la page exec=gis affiche en pleine largeur une carte ou la liste des points
- maj des libs pour corrections de bugs et permettre de tester la nouvelle feature du cluster cf Leaflet/Leaflet.markercluster#292


git-svn-id: svn://zone.spip.org/spip-zone/_plugins_/gis/trunk@80230 ac52e18a-acf5-0310-9fe8-c4428f23b10a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.