-
-
Notifications
You must be signed in to change notification settings - Fork 874
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
[FEATURE] Use classes instead of Record
s for efficiency
#1769
Comments
Record
s for efficiency
@josxha That would include this one in (double, double) projectXY(LatLng latlng); Would definitely be a breaking change, but would hopefully bring better performances. Something like that? DoublePoint projectDoublePoint(LatLng latlng); Instead of redundant code like this one: _ProjectedPolygon._fromPolygon(Projection projection, Polygon<R> polygon)
: this._(
polygon: polygon,
points: List<DoublePoint>.generate(
polygon.points.length,
(j) {
final (x, y) = projection.projectXY(polygon.points[j]);
return DoublePoint(x, y);
}, |
@josxha For the record I've just tried to replace
For next tests:
|
I will say that I don't think this is top priority. This is quite a level of micro-optimization! |
@josxha @monsieurtanuki quick update on this. I have been doing some numbers optimizations for another rendering project using FM and drawing verts. Here are a few things i have found that using the The math Point class is much slower (2-3x in my testing) than the Offset class which is fixed to the double type. I actually looked at swapping out all of the references to Point in FM to Offset, and it affects many classes but it should be doable, and would lead to noticeable improvements in performance, especially in slow areas. Avoid creating new objects, always reuse objects if possible. Record/destructuring response from the project method is significantly faster than the Writing/reading things to fixed size lists is insanely fast, easily 10x faster than Linked List types. I am able to render easily in under 16ms well over 50k projected triangles with unique colors per frame using DrawVertsRaw and these tricks above! I believe that implementing fixed size lists, using the destructuring/record type for offsets and removing offsets from the last 0-2 steps of rendering (depending on where it is appropriate) will lead to significant improvements in render time for the most expensive layers like polygons and polylines. Storing a Float32List for all positions is significantly faster than a List of Offsets. |
@mootw If relevant, we could even create our own MyPoint class. We somehow currently do so, with About fixed-length lists, it does look promising, but there may be problems with memory (as I assume they allocate a single consecutive space that needs to be "big enough", like an array, TBC). What happens when we add an item: does it rebuild the whole array? |
You cannot add items to fixed length arrays! So they aren't always the best solution. I have found they are best for storing and transforming a final list of points. Like, for instance, we take the origin point of the map and the list of projected points, iterating over that list, modifying, and allocating it, is going to be way faster than using a linked list. But a fixed size list likely will end up slower than a Linked list if you're constantly re-allocating it. Even if you use a separate length pointer, and allocate a fixed "largest possible size". I did a test implementation of one of the polyline simplifier functions (radial dist), using fixed size arrays), and it costs more than using the linked list in my testing with about 500,000 points, and a fairly large simplification factor, because we are "wasting" most of the reserved memory space. But in that case for a low simplification factor the cost is SIGNIFICANTLY less for the same function. Example:
ending points: 159
ending points: 500,000 You can see that the These techniques are i would argue BETTER than doing bit-packing or other weird language tricks and other stuff like that. (2 float 32s in a float64 for instance). Generally optimization like that should be handled by the compiler in my opinion, because the way we would do it would certainly be worse in some edge case. But something like using a fixed size list is a real optimization that has certain places where it can show its strengths and really make a difference because fundamentally its different than a linked list (also see HashMap vs Map). |
Thanks a lot for all the investigation @mootw!
I am a huge fan of removing math.Point in favor of something better, too. It would be sad if we throw all the simplification out. However, if an improved memory usage is better than we currently have with the simplification, it might be worth to roll with that.
Those typed_data types would fit great to accieve this goal, too:
typed_data lists allow to update in-place. In fact, wouldn't we even achieve 1 instead of N objects?
That's awesome! And as you said, especially for drawVertices we need it in this format in the end anyways. |
The research in #1750 (comment) and #1750 (comment) points out the reasons to avoid records.
Records have no advantage on memory consumtion while they require about double the time to be initialized.
As a consequence we should remove records from the package and use immutable classes in favour of it.
The text was updated successfully, but these errors were encountered: