This repository was archived by the owner on Dec 13, 2018. It is now read-only.
Don't set AsyncLocal for scope if not included#715
Closed
benaadams wants to merge 2 commits intoaspnet:devfrom
Closed
Don't set AsyncLocal for scope if not included#715benaadams wants to merge 2 commits intoaspnet:devfrom
benaadams wants to merge 2 commits intoaspnet:devfrom
Conversation
Member
|
Lets say you enable scopes after you're already in a scope, you now lose information for the logs in that scope until you leave and renter it. Not sure how important of a scenario that is though. |
Contributor
Author
The start events won't be in a scope either |
Contributor
|
Yeah, in that scenario, in-progress scopes are already messed up anyway. I don't think optimizations like this would cause any more problems. |
Contributor
Author
|
I think this has been resolved by resolved by #723? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't disturb the
ExecutionContextif scopes aren't being used as it wildly increases allocationsWithout ConsoleLogger (no changes to AsyncLocal)
With ConsoleLogger and no scopes - but not logging anything (changes to AsyncLocal)
You can see the allocations of
ActionandMoveNextRunnerdisproportionately increase; prior to this changeResolves #714
/cc @davidfowl
You are right @anurse, logging allocations make me sad 😢