logging icon indicating copy to clipboard operation
logging copied to clipboard

Make it easier to tree shake logging code

Open DartBot opened this issue 9 years ago • 9 comments

<img src="https://avatars.githubusercontent.com/u/5479?v=3" align="left" width="96" height="96"hspace="10"> 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 avatar Jun 05 '15 22:06 DartBot

<img src="https://avatars.githubusercontent.com/u/1924313?v=3" align="left" width="48" height="48"hspace="10"> Comment by jtmcdole


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

DartBot avatar Jun 05 '15 22:06 DartBot

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

donny-dont avatar Jan 22 '16 00:01 donny-dont

sure thing :)

sigmundch avatar Jan 22 '16 00:01 sigmundch

@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/logging/issues/51 where we imagine whether we should overhaul the entire API.

natebosch avatar Mar 12 '20 00:03 natebosch

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.

sigmundch avatar Mar 12 '20 22:03 sigmundch

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 [email protected] 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/logging/issues/20#issuecomment-598460551, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOVZWKV6CVCPAYD5GPVQCTRHFO6TANCNFSM4BHIKK6A .

jtmcdole avatar Mar 13 '20 02:03 jtmcdole

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

maximveksler avatar Aug 09 '22 15:08 maximveksler

cc @bkonyi re: the last question

devoncarew avatar Aug 09 '22 15:08 devoncarew

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.

bkonyi avatar Aug 09 '22 20:08 bkonyi