Minimal implementation of FlutterError.toString for release mode #54291
Conversation
fluttergithubbot
added
the
framework
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| @@ -890,6 +890,10 @@ class FlutterError extends Error with DiagnosticableTreeMixin implements Asserti | |||
|
|
|||
| @override | |||
| String toString({DiagnosticLevel minLevel = DiagnosticLevel.info}) { | |||
| if (kReleaseMode) { | |||
| final Iterable<_ErrorDiagnostic> errors = diagnostics.whereType<_ErrorDiagnostic>(); | |||
There was a problem hiding this comment.
How much does this increase binary size? That was the primary driver of this change
There was a problem hiding this comment.
Ideally we want to completely avoid retaining the diagnostics related classes
There was a problem hiding this comment.
This will slightly reduce the binary size. For the sample app in #54038 the total size of the segments in libapp.so is:
- 3611038 with this patch
- 3613222 without
The diagnostics classes are present in the release build, but their toString methods return an empty string. FlutterError.toString was passing the diagnostics nodes to the TextTreeRenderer, which just yields a series of empty strings. With this patch FlutterError.toString will return the value of the ErrorSummary, and the compiler can eliminate the TextTreeRenderer invocation.
Fixes #54038