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

Option for SuperClusterer algorithm bounding box #136

Closed
phil-chp opened this issue Dec 8, 2021 · 6 comments · Fixed by #640
Closed

Option for SuperClusterer algorithm bounding box #136

phil-chp opened this issue Dec 8, 2021 · 6 comments · Fixed by #640
Labels
released triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@phil-chp
Copy link

phil-chp commented Dec 8, 2021

Hello @jpoehnelt,

Is there a reason why 2 months ago (#60, src/algorithms/supercluster.ts:96), you switched the bounding box for getClusterer() in the SuperClusterer algorithm from a dynamic [west, south, east, north] to a static [-180, -90, 180, 90]?

We were working on a project, and we had some huge performance issues (tested on Ubuntu 20.04 LTS and macOS 12.1 Monterey).

Running on 2.3k markers, going above 100 clusters on the entire map would literally crash the browser.

Digging through the SuperClusterer algorithm, we found the bounding box set on those previously mentioned static numbers. Changing back those values for the dynamic ones, fixed it.

Correct me if I'm wrong, but it seems relevant to only want the algorithm to operate on currently visible clusters (instead of the entire world).

Thank you,
Have a lovely day.

(Related pull request : !135)

@phil-chp phil-chp added triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Dec 8, 2021
@jpoehnelt
Copy link
Contributor

jpoehnelt commented Dec 8, 2021

There is a trade off here that needs to be analyzed further.

Which is better? Might depend on the specifics of the data. If markers are concentrated in a small area, the current implementation is probably more performant. If the markers are more uniform, probably the latter implementation is better.

It is possible to pass your own algorithm to meet your specific data patterns.

@phil-chp
Copy link
Author

phil-chp commented Dec 9, 2021

That's an interesting point, we directly assumed it was a bug because of the browser crash and change in src/algorithms/supercluster.ts:96.

We have made another algorithm as suggested.

I'm interested in you mentioning that the current implementation is more optimized for concentrated data, because, that represents our case.
We were confronted with huge performance issues, using 2.3k markers in an approx. area of 30 km².


If you also feel divided by this, and understand the need for a dynamic bounding box in some cases, maybe it would be appealing to introduce it as a SuperClusterOptions?
(This goes hand-in-hand with extendBoundsToPaddedViewport(), so it doesn't feel too far-fetched to add it here.)

Some sort of :

enum bboxStatus {
    WHOLE_GLOBE,
    DYNAMIC,
    CUSTOM
}

function defineBoundingBox(status: bboxStatus = bboxStatus.WHOLE_GLOBE, custom?: number[]): number[] { ... }

If it were to be implemented and added in the docs, it would save lots of time for future people encountering this problem.

@jpoehnelt
Copy link
Contributor

Good suggestion on the configuration option.

@jpoehnelt jpoehnelt changed the title [PERFORMANCE ISSUE] SuperClusterer algorithm has entire world for bounding box? Option for SuperClusterer algorithm bounding box Dec 13, 2021
@davidpadych
Copy link

Hi, i have the same performance issue... it would help if this option existed in the supercluster options...
Do you plan to add this option in the next future version? thank you

@jpoehnelt
Copy link
Contributor

Please submit a pr! 😄

@googlemaps-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
4 participants