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

perf: micro-optimize Bounds #1706

Merged
merged 1 commit into from
Oct 27, 2023
Merged

perf: micro-optimize Bounds #1706

merged 1 commit into from
Oct 27, 2023

Conversation

ignatz
Copy link
Contributor

@ignatz ignatz commented Oct 27, 2023

It may be somewhat worth it just because Bounds are instantiated many times per frame. I might have seen some performance uplift qualitatively playing around in profile mode, and maybe fewer GC pauses but see for yourself or whether we consider this within margin 🤷

Screenshot_20231027-171344_flutter_map Demo

Screenshot_20231027-171244_flutter_map Demo

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Untested, but I think the auto tests have this under control. Thanks for the micro-optimizations, they're always appreciated!

@JaffaKetchup JaffaKetchup changed the title Micro optimization Bounds: do less and build less GC pressure. perf: micro-optimize Bounds Oct 27, 2023
@JaffaKetchup JaffaKetchup merged commit c24b6e6 into fleaflet:master Oct 27, 2023
6 checks passed
@bramp
Copy link
Contributor

bramp commented Oct 27, 2023

Flyby question. Surely this kind of optimisation is simple for the dart compiler to do, especially when constructors are constant. I'm 3 months into learning dart, so unsure how well the compiler works. Curious if you have more insight.

@JaffaKetchup
Copy link
Member

Tbh, I don't know how well the compiler does with these kinds of things either :D. But I would say that a function call and memory assignment still needs to maintain some kind of direct translation IYSWIM.
There's a place for some micro-optimizations and not others. This optimization is still very readable, and so I'm fine with it.

@ignatz
Copy link
Contributor Author

ignatz commented Oct 27, 2023

The optimizations here are more than just stylistish or helping the toolchain's optimizer. For example, whether you call Bounds() or Bounds._() has different assumptions. The former always sorts the points. It also shortens some code paths and require fewer objects to be initialized at handled at runtime by the GC.

Some languages, some toolchains do analyze the local scope and construct objects on the stack if they can't escape. For sure Go and Ocaml (maybe dart). Virtually none optimize beyond the local scope because it can quickly get out of hand. I'm only aware of Ocaml that has a few pending proposals to add "memory exclusivity" as a first-class feature to allow non-local optimizations in the future.

@ignatz
Copy link
Contributor Author

ignatz commented Oct 27, 2023

I should probably clarify that my prior hand-waving around whether the change is worth or not, was never about whether it saves cycles. It's more a question of whether the savings significant in the context of all the other work that goes into rendering a frame.

@josxha josxha added this to the v6.1 milestone Dec 2, 2023
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.

4 participants