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

Consider disabling by default or removing Chain.capture in build_runner #1039

Closed
kevmoo opened this issue Feb 22, 2018 · 4 comments
Closed
Labels
package:build_runner type-enhancement A request for a change that isn't a bug

Comments

@kevmoo
Copy link
Member

kevmoo commented Feb 22, 2018

Chain.capture(() async {

In one case, seems to add 8% to build times

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug S1 high package:build_runner labels Feb 22, 2018
@natebosch
Copy link
Member

+1 to remove

@jakemac53
Copy link
Contributor

Ya I am in favor of removing as well.

@natebosch
Copy link
Member

We had some discussion where we assumed that this was mainly capturing stack traces for exceptions which happen within the build_runner package, not within builders. When I look at #448 and it's related issue #427 it points to this helping with stack traces from exceptions within builders.

It's possible this is no longer the case if something has change in runBuilder, but we should check.

Worst case scenario is instead of remove it we default the when to false and add a flag to turn it back on specifically for Builder authors to diagnose issues.

The capture was added in #62 but there isn't discussion on why it was chosen instead of runZoned

@natebosch
Copy link
Member

OK, I think #550 effectively shifts the responsibility into runBuilder. We don't have the Chain.capture there, but probably don't need it since the stack trace should be much more focused at that point since it should really only be stuff in the builder code. Removing this altogether is right choice.

natebosch added a commit that referenced this issue Feb 23, 2018
Closes #1039

The stack traces for exceptions within builders are now captured and
logged from within `runBuilder`. The only exceptions that this chain
would capture are bugs in the build_runner package. Removing the
chaining shows a peformance improvement.
natebosch added a commit that referenced this issue Feb 23, 2018
Closes #1039

The stack traces for exceptions within builders are now captured and
logged from within `runBuilder`. The only exceptions that this chain
would capture are bugs in the build_runner package. Removing the
chaining shows a performance improvement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:build_runner type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants