Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it easier to tree shake logging code #20

Open
DartBot opened this issue Jun 5, 2015 · 9 comments
Open

Make it easier to tree shake logging code #20

DartBot opened this issue Jun 5, 2015 · 9 comments
Labels
type-question A question about expected behavior or functionality

Comments

@DartBot
Copy link

DartBot commented Jun 5, 2015

<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
Copy link
Author

DartBot commented Jun 5, 2015

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

@donny-dont
Copy link

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

@sigmundch
Copy link
Member

sure thing :)

@natebosch
Copy link
Member

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

@natebosch natebosch changed the title use compile-time environment constants to help turn off logging in production Make it easier to tree shake logging code Mar 12, 2020
@natebosch natebosch added type-question A question about expected behavior or functionality and removed type-enhancement A request for a change that isn't a bug labels Mar 12, 2020
@sigmundch
Copy link
Member

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
Copy link

jtmcdole commented Mar 13, 2020 via email

@maximveksler
Copy link

maximveksler commented Aug 9, 2022

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
Copy link
Member

cc @bkonyi re: the last question

@bkonyi
Copy link

bkonyi commented Aug 9, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-question A question about expected behavior or functionality
Projects
None yet
Development

No branches or pull requests

8 participants