Make it easier to tree shake logging code

[DartBot]

Issue by sethladd Originally opened as dart-lang/sdk#15593

---

For the logging package, we can use the new environment compile-time constants to help optimize at production time.

See also http://blog.sethladd.com/2013/12/compile-time-dead-code-elimination-with.html

See also the logging package: http://pub.dartlang.org/packages/logging

[DartBot]

Comment by jtmcdole

---

I 110% want this from a performance and codesize issues.

[donny-dont]

Would the logging package be open for a patch for this?

[sigmundch]

sure thing :)

[natebosch]

@sigmundch - can you comment on what type of design we would need in this package to support tree shaking of logging code in dart2js? It's not clear to me that we have any low hanging fruit here in terms of changes we can make that the compiler could use.

Depending on what type of design we'd need to use we can consider it with other stuff in https://github.com/dart-lang/core/issues/441 where we imagine whether we should overhaul the entire API.

[sigmundch]

Agree, we need to treat this as part of the redesign IMO.

I don't think I have a an exact answer at the moment because we are still debating many venues for what is a recommended way here:

• logging conditionally on some configuration flag/enum-value: the idea being that we can ensure the compiler can statically trace the value of this flag/enum-value and use it to determine whether some log lines. I hope this can be a bit more flexible than a String.fromEnvironment, though.
• kernel-level transformations that remove logging calls of certain level: this would requie the API to be very stable and statically recognizable so external tools can blindly delete log calls.
• something else that we haven't created: we've been discussing some annotation driven tree-shaking in dart2js (annotations used to check some code is only used for debugging and thus can be deleted in production). It's too early to say how that looks like, but might be worth discussing it.

[jtmcdole]

We had a const logger API (very very similar to this one) that I believe would treeshake better since you could tell at compile time if the logger would evaluate to true or not.

On Thu, Mar 12, 2020 at 3:30 PM sigmundch notifications@github.com wrote:

Agree, we need to treat this as part of the redesign IMO.

I don't think I have a an exact answer at the moment because we are still debating many venues for what is a recommended way here:

• logging conditionally on some configuration flag/enum-value: the idea being that we can ensure the compiler can statically trace the value of this flag/enum-value and use it to determine whether some log lines. I hope this can be a bit more flexible than a String.fromEnvironment, though.
• kernel-level transformations that remove logging calls of certain level: this would requie the API to be very stable and statically recognizable so external tools can blindly delete log calls.
• something else that we haven't created: we've been discussing some annotation driven tree-shaking in dart2js (annotations used to check some code is only used for debugging and thus can be deleted in production). It's too early to say how that looks like, but might be worth discussing it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dart-lang/core/issues/424, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOVZWKV6CVCPAYD5GPVQCTRHFO6TANCNFSM4BHIKK6A .

[maximveksler]

Guys,

Will dart:developer log() be tree shacked when kReleaseMode is true?

More specifically, will Object:Null() cause tree shaking? https://github.com/dart-lang/sdk/blob/e995cb5f7cd67d39c1ee4bdbe95c8241db36725f/runtime/lib/developer.cc#L47

[devoncarew]

cc @bkonyi re: the last question

[bkonyi]

cc @rmacnak-google will know for sure, but I'm guessing that calls to log() are not treeshaken in release builds. As you've already discovered, part of the log() implementation is in the VM code and just does nothing for Flutter release builds, but the compiler won't know anything about this and won't make any assumptions about whether or not log() is just a no-op, so that code will always be included in the application if it's referenced.

If you're concerned about logging being enabled in release mode for performance and/or memory reasons, your best bet is to use a global wrapper class around log() which wraps log() in an assert to make sure the code is only included in debug builds with asserts enabled.