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

Local state tracing instrumentation #66768

Merged
merged 15 commits into from
Feb 28, 2023
Merged

Local state tracing instrumentation #66768

merged 15 commits into from
Feb 28, 2023

Conversation

tmat
Copy link
Member

@tmat tmat commented Feb 9, 2023

The compiler currently allows for instrumentation of the entire assembly by setting EmitOptions.InstrumentationKinds. Only one kind is available today, which enables instrumentation for dynamic code coverage collection.

This change

  1. enables the caller of EmitDifference API to instruct the compiler to update a specific method symbol with a given set of instrumentations
  2. adds support for a new InstrumentationKind.LocalStateTracing.
  3. adds public API: EnC API for method instrumentation #66770

[1] is implemented by making the method compiler ask PEModuleBuilder for the instrumentations of the method being compiled. A builder used to compile entire assembly simple returns the value from EmitOptions, while the EnC builder uses the information passed to EmitDifference.
[2] A new LocalStateTracingInstrumenter is added to the chain of instrumenters applied during local rewrite.

This instrumenter adds calls to well-known instrumentation helpers to the bodies of methods. These allow tracing method entries, returns and writes to user-defined local variables and parameters. It also adds the ability to stitch calls to MoveNext methods of a state machine that are executed as continuations of the same instance of the state machine but potentially from multiple different threads.

The instrumentation introduces several new bound nodes:

BoundBlockInstrumentation

This node is attached to a BoundBlock that represents the lowered body of an instrumented method, lambda or local function. It defines a local variable used to store instrumentation context and prologue and epilogue.

BoundBlock with block instrumentation is eventually lowered to:

<prologue>
try
{
   <method body>
}
finally
{
   <epilogue>
}

The prologue is:

var $context = LocalStateTracker.LogXyzEntry(<ids>);

Where Xyz is a combination of StateMachine and either Method or Lambda, and is the corresponding set of arguments identifying the context.
The -Entry method is a static factory method for LocalStateTracker.
LocalStateTracker is a ref struct. It can only be allocated on the stack and accessed only directly from the declaring method (no lifting).
For member methods a single id is passed that is the method token.
For lambdas and local functions the token of the lambda method are passed in addition to the containing method token.
For state machines, the state machine instance id is passed. The instance id is stored on a new synthesized field of the state machine type added by the instrumentation that is initialized to a unique number when the state machine type is instantiated.

The epilogue is simply:

$context.LogReturn();

BoundStateMachineInstanceId

This node represents a reference to a synthesized state machine instance id field of the state machine. Lowered to a field read during state machine lowering.

BoundParameterId, BoundLocalId
Represents id of a user-defined parameter/local. Emitted as ldc.i4 of either the parameter/local ordinal if the variable was not lifted, or the token of its hoisted field (offset by 0x10000, the limit on number of method parameters/locals)

Each local variable write is followed by a call to one of the LogLocalStoreXyz or LogParameterStoreXyz instance methods on $context. The logger is passed either BoundParameterId or BoundLocalId and the current value of the local/parameter. The loggers are specialized to handle all kinds of variable types efficiently (without boxing).
Specialized loggers are used to track local variable aliases via ref assignments.

@tmat tmat requested review from a team as code owners February 9, 2023 01:37
@tmat tmat force-pushed the LST-Compiler branch 2 times, most recently from 6906704 to 680a9ac Compare February 9, 2023 04:17
@tmat
Copy link
Member Author

tmat commented Feb 9, 2023

@cston @jjonescz PTAL

Copy link
Member

@jjonescz jjonescz left a comment

Choose a reason for hiding this comment

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

Only partly reviewed. Will continue later.

@cston
Copy link
Member

cston commented Feb 17, 2023

        => (index >= 0x10000) ? $"P'{M.Module.ResolveField(index - 0x10000 + 0x04000000).Name}'" : $"P'{Parameters[index].Name}'[{index}]";

Should this call UnmangleFieldName()?


In reply to: 1433995302


In reply to: 1433995302


In reply to: 1433995302


Refers to: src/Compilers/CSharp/Test/Emit/Emit/LocalStateTracing/LocalStateTracingTests.cs:105 in 92ac7e5. [](commit_id = 92ac7e5, deletion_comment = False)

@cston
Copy link
Member

cston commented Feb 17, 2023

            expectedOutput: null);

expectedOutput?


In reply to: 1433998240


In reply to: 1433998240


In reply to: 1433998240


Refers to: src/Compilers/CSharp/Test/Emit/Emit/LocalStateTracing/LocalStateTracingTests.cs:182 in 92ac7e5. [](commit_id = 92ac7e5, deletion_comment = False)

@cston
Copy link
Member

cston commented Feb 17, 2023

        var diff = compilation1.EmitDifference(

What changed between f0 and f1?


In reply to: 1434005776


In reply to: 1434005776


In reply to: 1434005776


Refers to: src/Compilers/CSharp/Test/Emit/Emit/LocalStateTracing/LocalStateTracingTests.cs:406 in 92ac7e5. [](commit_id = 92ac7e5, deletion_comment = False)

@cston
Copy link
Member

cston commented Feb 17, 2023

        var expectedSize = (int)typeof(Unsafe).GetMethod("SizeOf")!.MakeGenericMethod(type)!.Invoke(null, Array.Empty<object>())!;

Consider using Unsafe.SizeOf(typeof(S2<int>)) directly in the test data, even if we need to extract the test data into a public member and use [MemberData(nameof(UnmanagedStruct_TestData))].


In reply to: 1434018635


In reply to: 1434018635


Refers to: src/Compilers/CSharp/Test/Emit/Emit/LocalStateTracing/LocalStateTracingTests.cs:1168 in 92ac7e5. [](commit_id = 92ac7e5, deletion_comment = False)

@cston
Copy link
Member

cston commented Feb 28, 2023

        options.TestOnly_AllowLocalStateTracing();

Consider adding if (kinds.Contains(InstrumentationKindExtensions.LocalStateTracing))


Refers to: src/Compilers/CSharp/Test/Emit/Emit/LocalStateTracing/LocalStateTracingTests.cs:34 in 0a5785f. [](commit_id = 0a5785f, deletion_comment = False)

Copy link
Member

@cston cston left a comment

Choose a reason for hiding this comment

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

Last two commits LGTM.

@tmat tmat enabled auto-merge (squash) February 28, 2023 19:14
@tmat tmat merged commit c94b8ca into dotnet:main Feb 28, 2023
@ghost ghost added this to the Next milestone Feb 28, 2023
@tmat tmat deleted the LST-Compiler branch February 28, 2023 22:33
@RikkiGibson RikkiGibson modified the milestones: Next, 17.6 P2 Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants