AOT treeshaker should remove unused fields #35310
Assignees
Comments
rmacnak-google
added
the
area-vm
|
Currently TFA does not seem to distinguish whether some field is only written to or both written to and read from. It seems if we add the distinction then tree shaking would become trivial. |
dart-bot
pushed a commit
that referenced
this issue
Apr 16, 2020
This relands commit ff34fd8, but puts tree shaking of write-only fields under the flag which is disabled by default. So far tree shaking was removing fields which are not used at all. This change improves tree shaking of fields so fields which are only written or used as interface targets can be removed. The following limitations apply: * Field is not removed if there is a constant object with that field, as it may impact identity of constant objects which is an observable behavior. * Instance field is not removed if it has a non-trivial initializer as it may have side-effects when executed by constructors. * Late final fields are not removed, as writing such fields may have side-effect. * When field is removed, we may need to introduce an abstract getter or abstract setter if field is used as a target of an interface call. If a field was written, then setter would be non-abstract (but empty). Issue #35310 Change-Id: Iec75b8301892664f4f955a01e5960b17e6620531 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/143286 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
dart-bot
pushed a commit
that referenced
this issue
May 21, 2020
Tree-shaking of write only fields is implemented in https://dart-review.googlesource.com/c/sdk/+/143286 This change just enables it by default. Flutter gallery in release mode armv7 app.so size -0.41% (gzip -0.24%, brotli -0.20%) armv8 app.so size -0.38% (gzip -0.23%, brotli -0.29%) Issue: #35310 Change-Id: Iaae3e893b4a6bf4d468d6cd05aba5f8ee8810afc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148543 Reviewed-by: Ryan Macnak <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
dart-bot
pushed a commit
that referenced
this issue
May 21, 2020
This reverts commit 9c50efc. Reason for revert: vm-kernel-precomp-win-release-x64 language_2/unsorted/disassemble_test test started to fail on this cl Original change's description: > [vm/aot] Enable tree-shaking of write-only fields by default > > Tree-shaking of write only fields is implemented in > https://dart-review.googlesource.com/c/sdk/+/143286 > This change just enables it by default. > > Flutter gallery in release mode > armv7 app.so size -0.41% (gzip -0.24%, brotli -0.20%) > armv8 app.so size -0.38% (gzip -0.23%, brotli -0.29%) > > Issue: #35310 > > Change-Id: Iaae3e893b4a6bf4d468d6cd05aba5f8ee8810afc > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148543 > Reviewed-by: Ryan Macnak <[email protected]> > Commit-Queue: Alexander Markov <[email protected]> [email protected],[email protected],[email protected],[email protected] Change-Id: Iafeb6c35eced8344fc86bae9c2b7617718158163 No-Presubmit: true No-Tree-Checks: true No-Try: true Issue: #35310 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148860 Reviewed-by: Alexander Aprelev <[email protected]>
dart-bot
pushed a commit
that referenced
this issue
May 27, 2020
This is a reland of 9c50efc There are no changes to the original CL. Fixes for bugs revealed by this change are in separate CLs: * Crash in disassembler https://dart-review.googlesource.com/c/sdk/+/148942 * Crash in --print_instructions_sizes_to https://dart-review.googlesource.com/c/sdk/+/149043 Original change's description: > [vm/aot] Enable tree-shaking of write-only fields by default > > Tree-shaking of write only fields is implemented in > https://dart-review.googlesource.com/c/sdk/+/143286 > This change just enables it by default. > > Flutter gallery in release mode > armv7 app.so size -0.41% (gzip -0.24%, brotli -0.20%) > armv8 app.so size -0.38% (gzip -0.23%, brotli -0.29%) > > Issue: #35310 > > Change-Id: Iaae3e893b4a6bf4d468d6cd05aba5f8ee8810afc > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148543 > Reviewed-by: Ryan Macnak <[email protected]> > Commit-Queue: Alexander Markov <[email protected]> Change-Id: I609db3fec8b0798f55aa2067127dfdc90a21ead4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149044 Reviewed-by: Vyacheslav Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A few classes in the Flutter framework have
_debug*fields, that are only accessed from within asserts. It would be cool if the treeshaker could remove these fields in release builds where they are not used and never accessed.This should help with reducing Flutter's memory footprint when running an app and it will also slightly reduce the AOT snapshot size on disk.
/cc @rmacnak-google
Related: #35303, if that's implemented even more fields are going to be unused in release mode.
The text was updated successfully, but these errors were encountered: