Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Reduce ExecutionContext register handling code size #21908

Closed
wants to merge 2 commits into from

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Jan 9, 2019

Brought up in #21875 (comment)

Remove executing on the ExecutionContext with error handling out of flow to RunReturningAnyException methods; away from setting and restoring ExecutionContext to reduce the need for manual enregistering workarounds.

Total bytes of diff: -1955 (-0.05% of base)
    diff is an improvement.

Total byte diff includes 2397 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    4 unique methods,     2397 unique bytes

Top file improvements by size (bytes):
       -1955 : System.Private.CoreLib.dasm (-0.05% of base)

1 total files with size differences (1 improved, 0 regressed), 0 unchanged.

Top method regessions by size (bytes):
        2074 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:RunReturningAnyException(byref):ref (0/25 methods)
         174 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,byref):ref (0/2 methods)
          84 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,ref):ref (0/1 methods)
          65 : System.Private.CoreLib.dasm - ExecutionContext:RestoreChangedContextToThread(ref,ref,ref) (0/1 methods)

Top method improvements by size (bytes):
       -3974 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:Start(byref) (25 methods)
        -168 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,byref) (2 methods)
        -129 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,ref)
         -81 : System.Private.CoreLib.dasm - ExecutionContext:RunFromThreadPoolDispatchLoop(ref,ref,ref,ref)

8 total methods with size differences (4 improved, 4 regressed), 17548 unchanged.

https://github.com/dotnet/coreclr/pull/21908/files?w=1

/cc @stephentoub @jkotas not sure if this is a help or hindrance?

@benaadams
Copy link
Member Author

Total bytes of diff: -1012 (-0.03% of base)
    diff is an improvement.

Total byte diff includes 2332 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    3 unique methods,     2332 unique bytes

Top file improvements by size (bytes):
       -1012 : System.Private.CoreLib.dasm (-0.03% of base)

1 total files with size differences (1 improved, 0 regressed), 0 unchanged.

Top method regessions by size (bytes):
        2074 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:RunReturningAnyException(byref):ref (0/25 methods)
         174 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,byref):ref (0/2 methods)
          84 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,ref):ref (0/1 methods)

Top method improvements by size (bytes):
       -3074 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:Start(byref) (25 methods)
         -96 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,byref) (2 methods)
         -93 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,ref)
         -81 : System.Private.CoreLib.dasm - ExecutionContext:RunFromThreadPoolDispatchLoop(ref,ref,ref,ref)

7 total methods with size differences (4 improved, 3 regressed), 17549 unchanged.

@benaadams
Copy link
Member Author

A little more with next commit

Total bytes of diff: -1955 (-0.05% of base)
    diff is an improvement.

Total byte diff includes 2397 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    4 unique methods,     2397 unique bytes

Top file improvements by size (bytes):
       -1955 : System.Private.CoreLib.dasm (-0.05% of base)

1 total files with size differences (1 improved, 0 regressed), 0 unchanged.

Top method regessions by size (bytes):
        2074 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:RunReturningAnyException(byref):ref (0/25 methods)
         174 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,byref):ref (0/2 methods)
          84 : System.Private.CoreLib.dasm - ExecutionContext:RunReturningAnyException(ref,ref):ref (0/1 methods)
          65 : System.Private.CoreLib.dasm - ExecutionContext:RestoreChangedContextToThread(ref,ref,ref) (0/1 methods)

Top method improvements by size (bytes):
       -3974 : System.Private.CoreLib.dasm - AsyncMethodBuilderCore:Start(byref) (25 methods)
        -168 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,byref) (2 methods)
        -129 : System.Private.CoreLib.dasm - ExecutionContext:RunInternal(ref,ref,ref)
         -81 : System.Private.CoreLib.dasm - ExecutionContext:RunFromThreadPoolDispatchLoop(ref,ref,ref,ref)

8 total methods with size differences (4 improved, 4 regressed), 17548 unchanged.

ExecutionContext currentExecutionCtx = currentThread.ExecutionContext;
if (previousExecutionCtx != currentExecutionCtx)
{
ExecutionContext.RestoreChangedContextToThread(currentThread, previousExecutionCtx, currentExecutionCtx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part (moving a part of the less common logic into non-generic ExecutionContext.RestoreChangedContextToThread) looks good to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me do that as by itself as a fresh PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -1064,6 +1048,29 @@ internal static void ThrowAsync(Exception exception, SynchronizationContext targ
}
}

private static ExceptionDispatchInfo RunReturningAnyException<TStateMachine>(ref TStateMachine stateMachine) where TStateMachine : IAsyncStateMachine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra generic method is questionable because of the extra runtime metadata that comes with every generic method.

I think there is a good idea here to just have a small try/catch instead of finally. What would the gains look like if the same small try/catch is inline in the caller?

@benaadams
Copy link
Member Author

Will revisit post #21909

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants