Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Apr 21, 2020

This change converts all Float64List matrices to Float32List at the dart:ui interface boundary. Internally, it only uses Float32List. Float32List requires less memory and is orders of magnitude faster to allocate, and it has sufficient precision as Flutter mobile engine and Skia use 32-bit floats anyway.

This change speeds up frame preroll by 50% on the bench_card_infinite_scroll benchmark.

For more details on Float64Array allocation in JS (which backs Float64List in Dart) see the following:

@yjbanov yjbanov marked this pull request as ready for review April 21, 2020 22:51
@yjbanov yjbanov requested a review from ferhatb April 21, 2020 22:51
@@ -7,6 +7,19 @@ part of engine;

typedef OnBenchmark = void Function(String name, num value);

typedef Action<R> = R Function();
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intend to have profiler.dart change in this PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove it once I rebase on top of #17852. I will rebase on top of it and remove the profiler changes here. I only needed it to verify the impact of the change on preroll, where we do a ton of vector math.

commitScene(_persistedScene);
_lastFrameScene = _persistedScene;
return SurfaceScene(_persistedScene.rootElement);
timeAction<void>('sceneBuilderPrerollFrame', () {
Copy link
Contributor

Choose a reason for hiding this comment

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

treeshake from release builds ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to tree-shake timeAction entirely without making the cost super-ugly. Open to ideas, but currently decided that it wasn't worth it. In release mode timeAction becomes:

R timeAction<R>(String label, Action<R> action) => action();

So the cost is one closure. Since SceneBuilder.build runs once per frame the difference is negligible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants