Constant propagation directly from MarkStep #1771
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
Add a test for the complex case
Don't store methods on stack in the same structure as methods which are already processed. Makes the processed methods structure more strongly typed.
Closed
3 tasks
sbomer
approved these changes
Jan 20, 2021
| void ProcessMethods (Collection<TypeDefinition> types) | ||
| { | ||
| foreach (var type in types) { | ||
| if (type.IsInterface) |
There was a problem hiding this comment.
Looks like we no longer check for interfaces - does it just mean that default interface methods can have branches removed?
There was a problem hiding this comment.
Good catch - and basically yes. I fixed it (unintentionally 😉 ). So I now added at least a basic test that branch removal works within default interface methods.
marek-safar
approved these changes
Jan 21, 2021
Add a test for branch removal in default interface methods
jeromelaban
added a commit
to unoplatform/Uno.DotnetRuntime.WebAssembly
that referenced
this pull request
Jan 26, 2021
Includes fixes to improve linking performance dotnet/linker#1771
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
Call constant propagation and branch removal processing directly from `MarkStep`: * Renames `RemoveUnreachableBlocksStep` to `UnreachableBlocksOptimizer` - it's not a step anymore * Rename members to use the `_name` naming convention (which is also used by `MarkStep`) * Removes the "go over all methods in all assemblies and process them" logic * Some small changes to the `ProcessMethod` API to be more suitable for the `MarkStep` * The class is now instantiated by `MarkStep` and only lives as long as `MarkStep` does * `MarkStep.ProcessMethod` calls the optimizer's `ProcessMethod` before doing anything else on the method (this is because the optimizer can modify methods body, so mark step should only look at the modified version of the method) Some interesting statistics (on console helloworld): * Before this change the optimizer processed 73K methods (every method at least once - total number of methods is 72K). After the change the optimizer only processes 12K methods (so a huge reduction). This also means that the large dictionary maintained by the optimizer on all processed methods is now 20% of before this change. * Max stack size increased to 62 - this is because of different order of processing methods. The 62 comes from `ThrowHelpers` which initializes lot of resource strings (each a separate call to a different method), so at one point all of these dependencies are on the stack. This is not a problem, just slightly higher memory usage. It would become problematic if the stack depth reached several 1000s which is VERY unlikely to happen. * Number of rewritten methods is down to 20% (from 746 to 117) - all of those not rewritten are actually not used by the app * As far as I can tell, it produces the exact same output. Very rough perf measurement on Blazor template app: * Before all the constant prop changes (SDK from early December 2020): 6.1 seconds (on average) * After this change: 3.4 seconds (on average) Commit migrated from dotnet/linker@d8c44b8
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.
Call constant propagation and branch removal processing directly from
MarkStep:RemoveUnreachableBlocksSteptoUnreachableBlocksOptimizer- it's not a step anymore_namenaming convention (which is also used byMarkStep)ProcessMethodAPI to be more suitable for theMarkStepMarkStepand only lives as long asMarkStepdoesMarkStep.ProcessMethodcalls the optimizer'sProcessMethodbefore doing anything else on the method (this is because the optimizer can modify methods body, so mark step should only look at the modified version of the method)Some interesting statistics (on console helloworld):
ThrowHelperswhich initializes lot of resource strings (each a separate call to a different method), so at one point all of these dependencies are on the stack. This is not a problem, just slightly higher memory usage. It would become problematic if the stack depth reached several 1000s which is VERY unlikely to happen.Very rough perf measurement on Blazor template app:
Part of #1733