Closed halvtomat closed 9 months ago
Hey @halvtomat
Thanks for the PR! I would prefer if we didn't have to introduce an ad-hoc API to fix a perf issue if we could perform some optimization(s) internally. The batching of the reads strategy from the gist you linked looks promising. Is that something we can pursue instead? https://gist.github.com/paulirish/5d52fb081b3570c81e3a#-appendix
I hear you and agree that batching would be preferred.
To make it possible, all markers would have to be centrally controlled so that we could tell them "calculate size" where they use the element.offsetHeight and element.offsetWidth attributes and then when all markers have completed the calculations tell them all to draw.
As of now, each marker is watching the map for changes and acting independently from each other, this makes batching difficult to coordinate between the markers.
To sum it up, in the current state I think quite a few changes would be required for batching to be possible. Do you think there is an alternative solution?
Acctually, I found another solution, new commit incoming.
@HusamIbrahim by adding the fastDOM library, all DOM reads and writes are batched, eliminating the layout reflow.
@halvtomat Added a couple of comments. Also curious if you tested this on your original issue and saw any noticeable improvement.
@HusamIbrahim I did test it and the performance was as good as my original improvement. The execution time of zooming in on a map with a few hundred markers went (on my machine) from ~170ms to ~18ms.
@halvtomat thanks for the contribution 👍🏻
Going forward can you please sign all commits? I'll get some pointers up on the site soonish to let people know.
@halvtomat can you resubmit a new PR when you're ready; sorry for the mess this morning
See issue #162
Significantly increases map performance.