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

[web] Start with a smaller memory allocation for CanvasKit #56900

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Dec 2, 2024

Confirmed that a sample Flutter app starts with a 32MB memory allocation for CanvasKit instead of 128MB.

Before After
image image

Fixes flutter/flutter#159499


Relevant emscripten settings:

Relevant emscripten code:

@mdebbar mdebbar requested a review from yjbanov December 2, 2024 18:27
@@ -170,7 +170,7 @@ canvaskit_wasm_lib("canvaskit") {
"-sFILESYSTEM=0",
"-sMODULARIZE",
"-sNO_EXIT_RUNTIME=1",
"-sINITIAL_MEMORY=128MB",
"-sINITIAL_MEMORY=64MB",
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if should try 32MB. Worst case it will cost one extra 32 => 64 reallocation, and that's not too big.

Copy link
Member

Choose a reason for hiding this comment

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

does this memory allocation size include textures and the onscreen render target? For reference, the initial heap size for just CPU allocations we use for Android is 4MB

Copy link
Member

Choose a reason for hiding this comment

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

oh this is for the entire wasm heap? maybe 4MB isn't quite enough...

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question. I imagine (hope?) that most textures and render targets bypass the wasm heap.

@chinmaygarde chinmaygarde added the platform-web Code specifically for the web engine label Dec 2, 2024
@mdebbar
Copy link
Contributor Author

mdebbar commented Dec 4, 2024

The default was to grow the memory by 20% each time. I changed it to grow by doubling to let rich apps reach their memory needs faster.

@mdebbar mdebbar requested a review from yjbanov December 4, 2024 21:13
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 4, 2024
@auto-submit auto-submit bot merged commit 7e23aa3 into flutter:main Dec 4, 2024
29 checks passed
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 5, 2024
…159818)

flutter/engine@9e8fcad...8d3c718

2024-12-04 [email protected] [Impeller] remove extra validation
checks in GLES backend. (flutter/engine#56944)
2024-12-04 [email protected] Remove LSAN supressions for Linux
embedder (flutter/engine#56913)
2024-12-04 [email protected] [web] Start with a smaller memory
allocation for CanvasKit (flutter/engine#56900)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@mdebbar mdebbar deleted the ck_less_memory branch December 5, 2024 16:58
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…ngine#56900)

Confirmed that a sample Flutter app starts with a 32MB memory allocation for CanvasKit instead of 128MB.

| Before | After|
|-|-|
| ![image](https://github.com/user-attachments/assets/5f6f2d4d-9155-499e-b20e-a15e3d8b53ab) | ![image](https://github.com/user-attachments/assets/bf1956c4-2d2a-44a8-b781-b110be93bc6a) |

Fixes flutter#159499

<hr>

Relevant emscripten settings:
- [`INITIAL_MEMORY`](https://emscripten.org/docs/tools_reference/settings_reference.html#initial-memory)
- [`ALLOW_MEMORY_GROWTH`](https://emscripten.org/docs/tools_reference/settings_reference.html#allow-memory-growth)
- [`MEMORY_GROWTH_GEOMETRIC_STEP`](https://emscripten.org/docs/tools_reference/settings_reference.html#memory-growth-geometric-step)
- [`MEMORY_GROWTH_GEOMETRIC_CAP`](https://emscripten.org/docs/tools_reference/settings_reference.html#memory-growth-geometric-cap)
- [`ABORTING_MALLOC`](https://emscripten.org/docs/tools_reference/settings_reference.html#aborting-malloc)

Relevant emscripten code:
- https://github.com/emscripten-core/emscripten/blob/58889f9f20f74d0dbaed7d49025c49b864359ae1/src/library.js#L290
- Emscripten tries to grow memory by the provided growth factor. If it fails, it tries to grow by 50% of that amount. If it fails, it tries 25%. Then it gives up and fails gracefully.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web] Reduce CanvasKit's initial memory allocation
4 participants