dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.24k stars 1.57k forks source link

Tree-shaking not producing the same result depending on whether Dart code is compiled as Javascript or Android binary #56766

Closed andynewman10 closed 1 month ago

andynewman10 commented 1 month ago

Create a Flutter application containing the following code:

import 'dart:math';

void log(String s) {
  // This line is basically 'if (!kReleaseMode)', but is also compatible with plain-Dart (not Flutter) compilation
  if (!(const bool.fromEnvironment('dart.vm.product'))) {
    print(s);
  }
}

final val = Random().nextInt(1000).toString();
log("Testlog: $val"); // only tree-shaken when building for web (Flutter or plain Dart)
log("Testsimplelog"); // tree-shaken in all cases (web or Android)

Now build with

Tree-shaking is less powerful with the Android compiler - is that expected, or is this a bug?

Note that, in both cases this time, "Testsimplelog" is correctly tree-shaken away.

Using Flutter 3.22.3 (Dart 3.4.4), on Windows.

Same result with Flutter 3.24.3 (Dart 3.5.3)

dart-github-bot commented 1 month ago

Summary: The issue reports that tree-shaking behavior differs between Flutter web and Android builds. String interpolation within a conditional log statement is tree-shaken in web builds but not in Android builds, even though a simpler log statement is tree-shaken in both cases. This suggests a potential discrepancy in tree-shaking implementation between the two platforms.

mraleph commented 1 month ago

Native compiler does not prune string interpolation nodes which are not used because interpolation can have a side-effect. We could replace _interpolate with something like _toString and prune string literals (and other types with known side-effect free toString) out of it.

andynewman10 commented 1 month ago

Thanks for your reply.

Tree shaking is an important step of pruning debug-related messages in the final binary, so it is very important to understand it.

I have the following code:

            final userCredential =
                await FirebaseAuth.instance.signInAnonymously();
            String? currentFauthid = userCredential.user!.uid!;
            String str = currentFauthid!.toString();
            final logString = "Firebase Auth id " + str; // not tree shaken. Why?
            log(logString);

            final val = Random().nextInt(1000).toString();
            log("Randomval1: $val"); // not tree shaken (confirms what you wrote)
            final logString2 = "Randomval2: " + val; // tree shaken
            log(logString2);

I have been banging my head on this for the last few hours, and I can't understand why, the 'logString' is not tree-shaken.

Still using the same log function listed at the top of this issue, building for android-arm.

mraleph commented 1 month ago

@andynewman10 I don't think there should be any difference between logString and logString2 in your example above. In both cases I would expect left hand side of the concatenation to stay in the binary.

andynewman10 commented 1 month ago

@mraleph you are correct, I did eventually find it in libapp.so. Sorry about this.

What can I do to have my log strings tree shaken when I am building the final app bundle? I wondered if there is maybe any advice you could give me?

This is really important that my debug strings are eliminated. Dart does not have a preprocessor and I don't even know if macros would solve this problem (I am not familiar with them and I understand they are still heavily in beta).

mraleph commented 1 month ago

@andynewman10 I think your best option for now is either to avoid using interpolation or to use closures:

const _absent = Object();

@pragma('vm:prefer-inline')
@pragma('dart2js:tryInline')
void log(
  String a, [
  Object? a0 = _absent,
  Object? a1 = _absent,
  Object? a2 = _absent,
  Object? a3 = _absent,
]) {
  if (!(const bool.fromEnvironment('dart.vm.product'))) {
    print([
      a,
      if (!identical(a0, _absent)) a0,
      if (!identical(a1, _absent)) a1,
      if (!identical(a2, _absent)) a2,
      if (!identical(a3, _absent)) a3,
    ].join(''));
  }
}

log("Firebase Auth id ", str)

or

@pragma('vm:prefer-inline')
@pragma('dart2js:tryInline')
void log(String Function() a) {
  if (!(const bool.fromEnvironment('dart.vm.product'))) {
    print(a());
  }
}

log(() => "Firebase Auth id $str");
andynewman10 commented 1 month ago

@mraleph your versions work with android-arm but - and that's sad - none of these 2 versions trigger tree-shaking when using a web target. I'm targetting both platforms (mobile and web) so I am still puzzled as to what I'm going to do to handle this. It indeed does not work in all cases, and because the syntax is always different, I am wondering what to do now.

andynewman10 commented 1 month ago

@mraleph It works. I had a proxy function inbetween log and my code in the web case, and this disrupted tree-shaking. I tried adding the pragmas on this function as well, but it didn't help. In the end I removed the proxy function which was of no importance. I think it is best if I check things on iOS, for completeness, before closing this issue.

sigmundch commented 1 month ago

Seems you resolved the issue by now, just wanted to note that the web compiler was probably behaving similar to the aot/arm builds also in the very first comment: the compiler needs to preserve the side-effects of the interpolation too. I believe the web output was removing the actual interpolated string, but included the side-effects from building it. For example, if you had:

int i = 0;
String nextValue() => "${i++}";

main() {
  log('entry: ${nextValue()} ${nextValue()}');
  print(i);
}

I would have expected this to print "2" in either debug or release mode.

Closures are one way to ensure those side-effects are omitted in release mode:

main() {
  log(() => 'entry: ${nextValue()} ${nextValue()}');
  print(i); // prints 0 in release mode.
}

Another option is to refactor the logging method, to make the log unconditional, but the presence of a logger conditional. For example:

class Logger {
  void log(s) => print(s);
}
final Logger? logger = const bool.fromEnvironment('dart.vm.product') ? null : Logger();

main() {
  logger?.log('entry: ${nextValue()} ${nextValue()}');
  print(i); // prints 0 in release mode.
}

Both web and native compilers recognize that the nullability of logger is basically constant, so they both manage to tree-shake all calls to ?.log as well.

I don't believe there are strong reasons to prefer one or the other option, it's just a matter of style and personal preference.

andynewman10 commented 1 month ago

@mraleph @sigmundch love the compactness yet flexibility of the closure, and the ?. operator trick is so simple and yet so smart, too. I verified that my iOS debug strings have been successfully pruned. I understand your points about interpolation, that needs to take place and "run". Thanks both for your valuable advice.