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

Conversation

eyebrowsoffire
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire commented Aug 23, 2023

This flag removes some code from CanvasKit to reduce size by a little bit. I went ahead and did a run of the benchmarks (flutter/flutter#133208) to see if it negatively affected anything, and there was no difference beyond noise between the current benchmark numbers and the benchmarks with this flag enabled.

The size differences are as follows:
Before the change:

total 30616
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:33 .
drwxr-xr-x   7 jacksongardner  primarygroup      224 Aug 10 18:14 ..
-rw-r--r--@  1 jacksongardner  primarygroup     6148 May 12 17:41 .DS_Store
-rw-r--r--   2 jacksongardner  primarygroup    94899 Aug 23 14:23 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  6631693 Aug 23 14:23 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  2102151 Aug 23 14:23 canvaskit.wasm.br
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:33 chromium
-rw-r--r--   2 jacksongardner  primarygroup   161478 Aug 23 14:28 skwasm.js
-rwxr-xr-x   2 jacksongardner  primarygroup  3296038 Aug 23 14:28 skwasm.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1101502 Aug 23 14:28 skwasm.wasm.br
-rw-r--r--   2 jacksongardner  primarygroup     3095 Aug 23 14:28 skwasm.worker.js

./chromium:
total 15520
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:33 .
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:33 ..
-rw-r--r--   2 jacksongardner  primarygroup    94545 Aug 23 14:25 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  5223378 Aug 23 14:25 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1492433 Aug 23 14:25 canvaskit.wasm.br

After the change:

total 28568
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:42 .
drwxr-xr-x   7 jacksongardner  primarygroup      224 Aug 10 18:14 ..
-rw-r--r--@  1 jacksongardner  primarygroup     6148 May 12 17:41 .DS_Store
-rw-r--r--   2 jacksongardner  primarygroup    94899 Aug 23 14:37 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  6401703 Aug 23 14:37 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  2038390 Aug 23 14:37 canvaskit.wasm.br
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:42 chromium
-rw-r--r--   2 jacksongardner  primarygroup   161478 Aug 23 14:41 skwasm.js
-rwxr-xr-x   2 jacksongardner  primarygroup  3143431 Aug 23 14:41 skwasm.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1050854 Aug 23 14:41 skwasm.wasm.br
-rw-r--r--   2 jacksongardner  primarygroup     3095 Aug 23 14:41 skwasm.worker.js

./chromium:
total 15392
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:42 .
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:42 ..
-rw-r--r--   2 jacksongardner  primarygroup    94545 Aug 23 14:39 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  4993586 Aug 23 14:39 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1427979 Aug 23 14:39 canvaskit.wasm.br

The brotli-compressed wasm modules save about 50-70kb each with this flag.

@eyebrowsoffire eyebrowsoffire marked this pull request as ready for review August 23, 2023 22:40
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #45029 at sha f079a2f

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, but be prepared to land golden updates downstream.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 24, 2023
@auto-submit auto-submit bot merged commit 143e3f3 into flutter:main Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
eyebrowsoffire added a commit that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
auto-submit bot pushed a commit that referenced this pull request Aug 24, 2023
… binary size" (#45082)

Reverts #45029

It appears this is causing a regression in the length of time some of our integration tests are taking, causing them to take twice as long, which is causing timeouts in CI. We should revert while we investigate.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 24, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 25, 2023
…133296)

flutter/engine@965501a...b8ec4da

2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from 99a76ea8e1b2 to 1428f16fc0de (1 revision) (flutter/engine#45086)
2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from 25fafff5b32c to 99a76ea8e1b2 (2 revisions) (flutter/engine#45083)
2023-08-24 jacksongardner@google.com Revert "Turn on the `skia_enable_optimize_size` flag to save a bit of binary size" (flutter/engine#45082)
2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from d7d56885a49b to 25fafff5b32c (1 revision) (flutter/engine#45081)
2023-08-24 dnfield@google.com [Impeller] Do not build scene unless 3d define is true (flutter/engine#45028)
2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from 177e8477faf9 to d7d56885a49b (1 revision) (flutter/engine#45078)
2023-08-24 dkwingsmt@users.noreply.github.com Reland: [Rasterizer] Make resubmit information temporary (flutter/engine#45037)
2023-08-24 34871572+gmackall@users.noreply.github.com Add case checking to android sdk cipd upload script (flutter/engine#45063)
2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from 007386294889 to 177e8477faf9 (1 revision) (flutter/engine#45076)
2023-08-24 jacksongardner@google.com Turn on the `skia_enable_optimize_size` flag to save a bit of binary size (flutter/engine#45029)
2023-08-24 skia-flutter-autoroll@skia.org Roll Skia from b17ee34f3378 to 007386294889 (1 revision) (flutter/engine#45075)

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 jimgraham@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…size (flutter#45029)

This flag removes some code from CanvasKit to reduce size by a little bit. I went ahead and did a run of the benchmarks (flutter/flutter#133208) to see if it negatively affected anything, and there was no difference beyond noise between the current benchmark numbers and the benchmarks with this flag enabled.

The size differences are as follows:
Before the change:
```
total 30616
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:33 .
drwxr-xr-x   7 jacksongardner  primarygroup      224 Aug 10 18:14 ..
-rw-r--r--@  1 jacksongardner  primarygroup     6148 May 12 17:41 .DS_Store
-rw-r--r--   2 jacksongardner  primarygroup    94899 Aug 23 14:23 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  6631693 Aug 23 14:23 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  2102151 Aug 23 14:23 canvaskit.wasm.br
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:33 chromium
-rw-r--r--   2 jacksongardner  primarygroup   161478 Aug 23 14:28 skwasm.js
-rwxr-xr-x   2 jacksongardner  primarygroup  3296038 Aug 23 14:28 skwasm.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1101502 Aug 23 14:28 skwasm.wasm.br
-rw-r--r--   2 jacksongardner  primarygroup     3095 Aug 23 14:28 skwasm.worker.js

./chromium:
total 15520
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:33 .
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:33 ..
-rw-r--r--   2 jacksongardner  primarygroup    94545 Aug 23 14:25 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  5223378 Aug 23 14:25 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1492433 Aug 23 14:25 canvaskit.wasm.br
```

After the change:
```
total 28568
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:42 .
drwxr-xr-x   7 jacksongardner  primarygroup      224 Aug 10 18:14 ..
-rw-r--r--@  1 jacksongardner  primarygroup     6148 May 12 17:41 .DS_Store
-rw-r--r--   2 jacksongardner  primarygroup    94899 Aug 23 14:37 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  6401703 Aug 23 14:37 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  2038390 Aug 23 14:37 canvaskit.wasm.br
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:42 chromium
-rw-r--r--   2 jacksongardner  primarygroup   161478 Aug 23 14:41 skwasm.js
-rwxr-xr-x   2 jacksongardner  primarygroup  3143431 Aug 23 14:41 skwasm.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1050854 Aug 23 14:41 skwasm.wasm.br
-rw-r--r--   2 jacksongardner  primarygroup     3095 Aug 23 14:41 skwasm.worker.js

./chromium:
total 15392
drwxr-xr-x   5 jacksongardner  primarygroup      160 Aug 23 14:42 .
drwxr-xr-x  11 jacksongardner  primarygroup      352 Aug 23 14:42 ..
-rw-r--r--   2 jacksongardner  primarygroup    94545 Aug 23 14:39 canvaskit.js
-rwxr-xr-x   2 jacksongardner  primarygroup  4993586 Aug 23 14:39 canvaskit.wasm
-rwxr-xr-x   1 jacksongardner  primarygroup  1427979 Aug 23 14:39 canvaskit.wasm.br
```

The brotli-compressed wasm modules save about 50-70kb each with this flag.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
… binary size" (flutter#45082)

Reverts flutter#45029

It appears this is causing a regression in the length of time some of our integration tests are taking, causing them to take twice as long, which is causing timeouts in CI. We should revert while we investigate.
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 will affect goldens

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants