search

dart-lang / logging

A Dart package for debug and error logging.
https://pub.dev/packages/logging
BSD 3-Clause "New" or "Revised" License
332 stars 52 forks source link

Make it easier to tree shake logging code #20

Open DartBot opened 9 years ago

DartBot commented 9 years ago

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 commented 9 years ago

Comment by jtmcdole

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

donny-dont commented 8 years ago

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

sigmundch commented 8 years ago

sure thing :)

natebosch commented 4 years ago

@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.

sigmundch commented 4 years ago

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:

jtmcdole commented 4 years ago

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

maximveksler commented 2 years ago

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 commented 2 years ago

cc @bkonyi re: the last question

bkonyi commented 2 years ago

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.