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

Optimize markers calculations #826

Merged
merged 11 commits into from
Apr 27, 2021
Merged

Conversation

TheLastGimbus
Copy link
Contributor

@TheLastGimbus TheLastGimbus commented Feb 21, 2021

This PR is attempting to optimize and shrink all calculations that are done when user drags map with a lot of markers

Hopefully, I will get it to state where we have full 60 fps with 5k markers, but we will se how it goes

Fixes #824 I would really want to Yeah, pretty much - I don't see we how could do much better than this

@TheLastGimbus
Copy link
Contributor Author

TheLastGimbus commented Feb 21, 2021

This CI is broken...

@TheLastGimbus
Copy link
Contributor Author

You literally commented that when I was about to push it 😄

Copy link
Contributor Author

@TheLastGimbus TheLastGimbus left a comment

Choose a reason for hiding this comment

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

This may look kinda hack-ish, but this actually brough whole loop's time (on my tablet) from ~20 to ~10ms! On 5k markers!

(As always: When zoomed to no markers and not zooming in/out)

@TheLastGimbus
Copy link
Contributor Author

@ibrierley You proposed to cache them per-zoom-level. Out of curiosity, I printed when zoom changes and:

I/flutter (16890): 6.249008887924742
I/flutter (16890): 6.254358513273377
I/flutter (16890): 6.255482934237276
I/flutter (16890): 6.261930705947585
I/flutter (16890): 6.266114410749081
I/flutter (16890): 6.2766681718506305
I/flutter (16890): 6.283982456742614
I/flutter (16890): 6.2892041742113065
I/flutter (16890): 6.296456503011495
I/flutter (16890): 6.301633471546501
I/flutter (16890): 6.303673583816311
I/flutter (16890): 6.30882487202124
I/flutter (16890): 6.311932043194194
I/flutter (16890): 6.317053908214326
I/flutter (16890): 6.3190726270623445

We probably couldn't phisically fit so many ints into whole device ram 😆 So I guess that's the best we can settle on

@TheLastGimbus
Copy link
Contributor Author

TheLastGimbus commented Feb 21, 2021

Okay, this is ready for review and merge

Hopefully, I will get it to state where we have full 60 fps with 5k

I have 30 fps at 5k markers and 60 at 2.5k - not bad 👍

@TheLastGimbus
Copy link
Contributor Author

One important thing I noticed:

When you have a simple widget as markers, and make it a const, fpses can sometimes double when you have all of them on the screen

We could add this hint somewhere in the documentation

(but, of course, behavior between devices/widget cases can vary)

lib/src/layer/marker_layer.dart Outdated Show resolved Hide resolved
lib/src/layer/marker_layer.dart Outdated Show resolved Hide resolved
- change to stateful
- make loop nicer
@KlausJokisuo
Copy link

Any news on this?

@TheLastGimbus
Copy link
Contributor Author

I was just about to apologize that I'm running late with pushing commits as always, but, oh, it's not me 😄

We are just waiting for @johnpryan review 👍

@KlausJokisuo
Copy link

KlausJokisuo commented Apr 15, 2021

I was just about to apologize that I'm running late with pushing commits as always, but, oh, it's not me 😄

We are just waiting for @johnpryan review 👍

No worries mate, what kind of performance improvements can except for example with 2000 markers and 200 visible.

@TheLastGimbus
Copy link
Contributor Author

TheLastGimbus commented Apr 15, 2021

Yeah, not that much, but it's always something

I think we need to make some new plugin for markers that will base on Canvas or something for that many markers

Edit: Oh, if that was a question - on my tablet, 3,5k markers where ~20fps when zoomed to 30 visible, and like 15fps when 200 visible. After optimization, 3.5k 60fps when <100 visible, then maybe 45fps when 200 visible 👍

@KlausJokisuo
Copy link

KlausJokisuo commented Apr 15, 2021

Yeah, not that much, but it's always something

I think we need to make some new plugin for markers that will base on Canvas or something for that many markers

Edit: Oh, if that was a question - on my tablet, 3,5k markers where ~20fps when zoomed to 30 visible, and like 15fps when 200 visible. After optimization, 3.5k 60fps when <100 visible, then maybe 45fps when 200 visible 👍

Thanks! Keep up the good work and sorry I phrased my question oddly. 😅

Copy link
Contributor

@kengu kengu left a comment

Choose a reason for hiding this comment

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

All RFCs from @johnpryan are resolved in code. LGTM.

@kengu
Copy link
Contributor

kengu commented Apr 25, 2021

@TheLastGimbus The only remaining thing to do is to resolve the conflicts with master in lib/src/layer/marker_layer.dart

# Conflicts:
#	lib/src/layer/marker_layer.dart
Or my IDE is just useless
@TheLastGimbus
Copy link
Contributor Author

Done! All working, markers rotating correctly 👌 @kengu ready to merge 👍

@kengu
Copy link
Contributor

kengu commented Apr 26, 2021

@TheLastGimbus I got this error in the log when opening the "A lot of markers" page in the example-app.

======== Exception caught by widgets library =======================================================
The following RangeError was thrown building ManyMarkersPage(dirty, state: _ManyMarkersPageState#bb88c):
RangeError (end): Invalid value: Only valid value is 0: 500

The relevant error-causing widget was: 
  ManyMarkersPage file:///Users/kengu/src/git/flutter_map/example/lib/main.dart:66:45
When the exception was thrown, this was the stack: 
#0      RangeError.checkValidRange (dart:core/errors.dart:333:9)
#1      List.sublist (dart:core-patch/growable_array.dart:84:38)
#2      _ManyMarkersPageState.build (package:flutter_map_example/pages/many_markers.dart:85:56)
#3      StatefulElement.build (package:flutter/src/widgets/framework.dart:4612:27)
#4      ComponentElement.performRebuild (package:flutter/src/widgets/framework.dart:4495:15)

This fixes it

allMarkers.sublist(0, min(allMarkers.length, _sliderVal))

@johnpryan I've testet this on my phone and it looks good. The merge is blocked by your request for change, which I think is mostly resolved.

@TheLastGimbus
Copy link
Contributor Author

Done 💯

@TheLastGimbus
Copy link
Contributor Author

@kengu you can pretty much press merge 👌
I think

@kengu
Copy link
Contributor

kengu commented Apr 27, 2021

@kengu you can pretty much press merge 👌
I think

Waiting on @johnpryan - unable to merge until changes are ack-ed by him

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.

Map gets super laggy when there are a lot of Markers
5 participants