make formatting performance better #3247
Merged
Conversation
This file contains 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
this performance improvement is particularly for devdiv bug # 1089540 this makes the file in the bug to be formatted in several seconds compared to several minutes on my machine. there were several issues. each one fixed by #1, use concurrency on gathering operations. #2, don't use too much time to split work to chunks if that requires more work than actually formatting. #3, don't blindly set beginning of a file as inseparable start point for certain formatting options. ... but these don't actually address the most impactful root cause of this perf issues. which is perf issue of GetPrevious/GetNextToken API in compiler. (dotnet#3244) formatter internally uses GetDescendantTokens to get all tokens at once and cache them which takes less than 1 second for the entire file (2M bytes size) in the bug. and use the cache internally. but certain part of formatter (Rule Provider) can't use that internal cache, so it has to use the GetPrevious/GetNextToken to move around tokens, which in this particular bug, takes more than 40 seconds on my machine. and that is not even for entire file. (less than 1/12 of tokens) I opened a bug to compiler team, hopely so that we can get better perf on those APIs. in this PR, I mitigated the issue either by making more things to run concurrently or by changing logic which requires those APIs.
|
related to #3229 |
|
@mattwar @jmarolf @dpoeschl @Pilchie @balajikris can you take a look? thank you. |
|
missed @rchande |
|
@dotnet-bot retest this please |
|
retest this please |
|
👍 |
|
retest this please |
|
retest this please |
heejaechang
added a commit
that referenced
this pull request
Jun 4, 2015
make formatting performance better ... this performance improvement is particularly for devdiv bug # 1089540 this makes the file in the bug to be formatted in several seconds compared to several minutes on my machine. there were several issues. each one fixed by #1, use concurrency on gathering operations. #2, don't use too much time to split work to chunks if that requires more work than actually formatting. #3, don't blindly set beginning of a file as inseparable start point for certain formatting options. ... but these don't actually address the most impactful root cause of this perf issues. which is perf issue of GetPrevious/GetNextToken API in compiler. (#3244) formatter internally uses GetDescendantTokens to get all tokens at once and cache them which takes less than 1 second for the entire file (2M bytes size) in the bug. and use the cache internally. but certain part of formatter (Rule Provider) can't use that internal cache, so it has to use the GetPrevious/GetNextToken to move around tokens, which in this particular bug, takes more than 40 seconds on my machine. and that is not even for entire file. (less than 1/12 of tokens) I opened a bug to compiler team, hopely so that we can get better perf on those APIs. in this PR, I mitigated the issue either by making more things to run concurrently or by changing logic which requires those APIs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
this performance improvement is particularly for devdiv bug # 1089540
this makes the file in the bug to be formatted in several seconds compared to several minutes on my machine.
there were several issues. each one fixed by
#1, use concurrency on gathering operations.
#2, don't use too much time to split work to chunks if that requires more work than actually formatting.
#3, don't blindly set beginning of a file as inseparable start point for certain formatting options.
...
but these don't actually address the most impactful root cause of this perf issues. which is perf issue of GetPrevious/GetNextToken API in compiler.
(#3244)
formatter internally uses GetDescendantTokens to get all tokens at once and cache them which takes less than 1 second for the entire file (2M bytes size) in the bug. and use the cache internally.
but certain part of formatter (Rule Provider) can't use that internal cache, so it has to use the GetPrevious/GetNextToken to move around tokens, which in this particular bug, takes more than 40 seconds on my machine. and that is not even for entire file. (less than 1/12 of tokens)
I opened a bug to compiler team, hopely so that we can get better perf on those APIs.
in this PR, I mitigated the issue either by making more things to run concurrently or by changing logic which requires those APIs.