-
Notifications
You must be signed in to change notification settings - Fork 8.1k
PowerShell profiler #13673
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
PowerShell profiler #13673
Changes from all commits
ade6b54
cabadd5
49e48f8
84b4629
f93bd8f
f3e3129
354a41b
1f17573
12760c8
a9ae2a2
7c631e4
f8ef7a4
dc344ef
2f84f19
03ff234
a7d6abb
8899bb6
483b2ea
9bdc763
4867a45
3d260ba
04bcd4c
ffdfc91
83b72d3
bf38033
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,7 +188,10 @@ public override SessionStateEntryVisibility Visibility | |
| { | ||
| get | ||
| { | ||
| if (Context == null) return SessionStateEntryVisibility.Public; | ||
| if (Context == null) | ||
| { | ||
| return SessionStateEntryVisibility.Public; | ||
| } | ||
|
|
||
| return Context.EngineSessionState.CheckScriptVisibility(_path); | ||
| } | ||
|
|
@@ -267,7 +270,11 @@ internal ScriptBlockAst GetScriptBlockAst() | |
| var scriptContents = ScriptContents; | ||
| if (_scriptBlock == null) | ||
| { | ||
| this.ScriptBlock = ScriptBlock.TryGetCachedScriptBlock(_path, scriptContents); | ||
| CompiledScriptBlockData compiledScriptBlockData = ScriptBlock.TryGetCachedCompiledScriptBlock(_path, scriptContents); | ||
| if (compiledScriptBlockData != null) | ||
| { | ||
| this.ScriptBlock = new ScriptBlock(compiledScriptBlockData); | ||
| } | ||
| } | ||
|
|
||
| if (_scriptBlock != null) | ||
|
|
@@ -305,7 +312,7 @@ internal ScriptBlockAst GetScriptBlockAst() | |
| if (errors.Length == 0) | ||
| { | ||
| this.ScriptBlock = new ScriptBlock(_scriptBlockAst, isFilter: false); | ||
| ScriptBlock.CacheScriptBlock(_scriptBlock.Clone(), _path, scriptContents); | ||
| ScriptBlock.CacheCompiledScriptBlock(_scriptBlock.Clone(), _path, scriptContents); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5269,6 +5269,7 @@ private static void InitializeCoreCmdletsAndProviders( | |
| { "Import-Module", new SessionStateCmdletEntry("Import-Module", typeof(ImportModuleCommand), helpFile) }, | ||
| { "Invoke-Command", new SessionStateCmdletEntry("Invoke-Command", typeof(InvokeCommandCommand), helpFile) }, | ||
| { "Invoke-History", new SessionStateCmdletEntry("Invoke-History", typeof(InvokeHistoryCommand), helpFile) }, | ||
| { "Measure-Script", new SessionStateCmdletEntry("Measure-Script", typeof(MeasureScriptCommand), helpFile) }, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree.
So using |
||
| { "New-Module", new SessionStateCmdletEntry("New-Module", typeof(NewModuleCommand), helpFile) }, | ||
| { "New-ModuleManifest", new SessionStateCmdletEntry("New-ModuleManifest", typeof(NewModuleManifestCommand), helpFile) }, | ||
| { "New-PSRoleCapabilityFile", new SessionStateCmdletEntry("New-PSRoleCapabilityFile", typeof(NewPSRoleCapabilityFileCommand), helpFile) }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1135,6 +1135,16 @@ internal void EnterScriptFunction(FunctionContext functionContext) | |
| Diagnostics.Assert(functionContext._executionContext == _context, "Wrong debugger is being used."); | ||
|
|
||
| var invocationInfo = (InvocationInfo)functionContext._localsTuple.GetAutomaticVariable(AutomaticVariable.MyInvocation); | ||
|
|
||
| if (ProfilerEventSource.LogInstance.IsEnabled()) | ||
| { | ||
| ProfilerEventSource.LogInstance.SequencePoint( | ||
| functionContext._scriptBlock?.Id ?? Guid.Empty, | ||
| functionContext._executionContext.CurrentRunspace.InstanceId, | ||
| _callStack.Last()?.FunctionContext._scriptBlock.Id ?? Guid.Empty, | ||
| 0); | ||
| } | ||
|
|
||
| var newCallStackInfo = new CallStackInfo | ||
| { | ||
| InvocationInfo = invocationInfo, | ||
|
|
@@ -1232,6 +1242,25 @@ internal void RegisterScriptFile(string path, string scriptContents) | |
| } | ||
| } | ||
|
|
||
| internal Guid GetParentScriptBlockId(int sequencePointPosition) | ||
| { | ||
| // Sequence point on 0 position is an entry point in a scriptblock. | ||
| // In the time the callstack has still point to parent scriptblock | ||
| // so we take last element from the callstack, | ||
| // for rest sequence points we take an element before last as parent. | ||
|
||
| int shift = sequencePointPosition == 0 ? 1 : 2; | ||
| if (_callStack.Count - shift >= 0) | ||
| { | ||
| ScriptBlock scriptBlock = _callStack[_callStack.Count - shift].FunctionContext._scriptBlock; | ||
| if (scriptBlock is not null) | ||
| { | ||
| return scriptBlock.Id; | ||
| } | ||
| } | ||
|
|
||
| return Guid.Empty; | ||
| } | ||
|
|
||
| #endregion Call stack management | ||
|
|
||
| #region setting breakpoints | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,15 +29,13 @@ private UpdatePositionInstruction(bool checkBreakpoints, int sequencePoint) | |
| public override int Run(InterpretedFrame frame) | ||
| { | ||
| var functionContext = frame.FunctionContext; | ||
| var context = frame.ExecutionContext; | ||
|
|
||
| functionContext._currentSequencePointIndex = _sequencePoint; | ||
| if (_checkBreakpoints) | ||
| { | ||
| if (context._debuggingMode > 0) | ||
| { | ||
| context.Debugger.OnSequencePointHit(functionContext); | ||
| } | ||
| functionContext.UpdatePosition(_sequencePoint); | ||
| } | ||
| else | ||
| { | ||
| functionContext.UpdatePositionNoBreak(_sequencePoint); | ||
| } | ||
|
Comment on lines
-32
to
39
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be changed to var functionContext = frame.FunctionContext;
functionContext._currentSequencePointIndex = _sequencePoint;
if (_checkBreakpoints)
{
functionContext.UpdatePosition(_sequencePoint);
} |
||
|
|
||
| return +1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -241,6 +241,12 @@ internal static class CachedReflectionInfo | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal static readonly MethodInfo FunctionContext_PushTrapHandlers = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof(FunctionContext).GetMethod(nameof(FunctionContext.PushTrapHandlers), InstanceFlags); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal static readonly MethodInfo FunctionContext_UpdatePosition = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof(FunctionContext).GetMethod(nameof(FunctionContext.UpdatePosition), InstanceFlags); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal static readonly MethodInfo FunctionContext_UpdatePositionNoBreak = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof(FunctionContext).GetMethod(nameof(FunctionContext.UpdatePositionNoBreak), InstanceFlags); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal static readonly MethodInfo FunctionOps_DefineFunction = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof(FunctionOps).GetMethod(nameof(FunctionOps.DefineFunction), StaticFlags); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -802,6 +808,30 @@ internal void PopTrapHandlers() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _traps.RemoveAt(_traps.Count - 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal void UpdatePositionNoBreak(int pos) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _currentSequencePointIndex = pos; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, we only emit events when Also, then this method doesn't really do anything and should be removed. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal void UpdatePosition(int pos) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UpdatePositionNoBreak(pos); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (ProfilerEventSource.LogInstance.IsEnabled()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ProfilerEventSource.LogInstance.SequencePoint( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _scriptBlock?.Id ?? Guid.Empty, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _executionContext.CurrentRunspace.InstanceId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _executionContext.Debugger.GetParentScriptBlockId(pos), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pos); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_executionContext._debuggingMode > 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _executionContext.Debugger.OnSequencePointHit(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+817
to
+834
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| internal class Compiler : ICustomAstVisitor2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -2145,13 +2175,28 @@ private static object GetExpressionValue( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var pipe = new Pipe(resultList); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ScriptBlock scriptBlock = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (ProfilerEventSource.LogInstance.IsEnabled()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // We need a scriptblock Id to keep track of the call stack. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scriptBlock = ScriptBlock.Create(context, expressionAst.Extent.Text); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // No need to optimize - we only want to add to the scriptblock cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // so that we can get rundown events. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // We can safely do this because lambda and sequencePoints | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // parameters of the method come always from callsites as null. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (We could remove them at all.) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scriptBlock.Compile(optimized: false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this for getting the script call stack when generating events? It looks quite hacky. I think we need to define the goal for this PR. I guess the goal is to make the Pester's coverage functionality able to work with the ETW events generated from the changes here. So, is the script stack necessary for the Pester's coverage functionality to work? If we set that as the goal, we should only include features/changes that are necessary for that to work.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both Pester code coverage, and Profiler would depend on this. Pester code coverage is a subset of Profiler functionality. Both use this interface to provide their services: https://github.com/nohwnd/Profiler/blob/main/csharp/Profiler/ITracer.cs Which collects: // The currently executing scriptblock and extent // how many items are on callstack // the current function and module name The data is collected here, by reflecting on debugger, functionContext and session state: https://github.com/nohwnd/Profiler/blob/main/csharp/Profiler/Tracer.cs#L157-L171 As long as this info is provided in some form, the current version of Profiler can work with this. "Level" I should be able to get from scriptblock ids as pointed out here: #13673 (comment) Module I should be able to get from the scriptblock and its associated session state. I think. But what about the function name? Any suggestions where I can grab that? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var functionContext = new FunctionContext | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _sequencePoints = sequencePoints, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _executionContext = context, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _file = expressionAst.Extent.File, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _outputPipe = pipe, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _localsTuple = MutableTuple.MakeTuple(localsTupleType, DottedLocalsNameIndexMap) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _localsTuple = MutableTuple.MakeTuple(localsTupleType, DottedLocalsNameIndexMap), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _scriptBlock = scriptBlock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (usingValues != null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -7055,26 +7100,10 @@ public override Expression Reduce() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exprs.Add(Expression.DebugInfo(_debugSymbolDocument, _extent.StartLineNumber, _extent.StartColumnNumber, _extent.EndLineNumber, _extent.EndColumnNumber)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exprs.Add( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.Assign( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.Field(Compiler.s_functionContext, CachedReflectionInfo.FunctionContext__currentSequencePointIndex), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExpressionCache.Constant(_sequencePoint))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_checkBreakpoints) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exprs.Add( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.IfThen( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.GreaterThan( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.Field(Compiler.s_executionContextParameter, CachedReflectionInfo.ExecutionContext_DebuggingMode), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExpressionCache.Constant(0)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.Call( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Expression.Field(Compiler.s_executionContextParameter, CachedReflectionInfo.ExecutionContext_Debugger), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CachedReflectionInfo.Debugger_OnSequencePointHit, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Compiler.s_functionContext))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exprs.Add(ExpressionCache.Empty); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MethodInfo method = _checkBreakpoints | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? CachedReflectionInfo.FunctionContext_UpdatePosition | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : CachedReflectionInfo.FunctionContext_UpdatePositionNoBreak; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exprs.Add(Expression.Call(Compiler.s_functionContext, method, ExpressionCache.Constant(_sequencePoint))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+7103
to
+7106
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be changed to: exprs.Add(
Expression.Assign(
Expression.Field(Compiler.s_functionContext, CachedReflectionInfo.FunctionContext__currentSequencePointIndex),
ExpressionCache.Constant(_sequencePoint)));
if (_checkBreakpoints)
{
exprs.Add(Expression.Call(Compiler.s_functionContext, CachedReflectionInfo.FunctionContext_UpdatePosition);
}
exprs.Add(ExpressionCache.Empty); |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Expression.Block(exprs); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,18 +39,46 @@ internal enum ScriptBlockClauseToInvoke | |
|
|
||
| internal class CompiledScriptBlockData | ||
| { | ||
| private static readonly ConcurrentDictionary<CompiledScriptBlockData, CompiledScriptBlockData> s_compiledScriptBlockTable | ||
| = new ConcurrentDictionary<CompiledScriptBlockData, CompiledScriptBlockData>(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do the key and value are the same object? Comments are needed here to explain the intent. |
||
|
|
||
| internal static void ClearCompiledScriptBlockTable() | ||
| { | ||
| s_compiledScriptBlockTable.Clear(); | ||
| } | ||
|
|
||
| internal static ICollection<CompiledScriptBlockData> GetCompiledScriptBlockList() | ||
daxian-dbw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| return s_compiledScriptBlockTable.Values; | ||
| } | ||
|
|
||
| internal static void InitCompiledScriptBlockTable() | ||
| { | ||
| ClearCompiledScriptBlockTable(); | ||
|
|
||
| foreach (CompiledScriptBlockData csb in ScriptBlock.GetCachedCompiledScriptBlockData()) | ||
| { | ||
| s_compiledScriptBlockTable.TryAdd(csb, csb); | ||
| } | ||
| } | ||
|
|
||
| internal void RegisterCompiledScriptBlockData() | ||
| { | ||
| s_compiledScriptBlockTable.TryAdd(this, this); | ||
| } | ||
|
|
||
| internal CompiledScriptBlockData(IParameterMetadataProvider ast, bool isFilter) | ||
| { | ||
| _ast = ast; | ||
| this.IsFilter = isFilter; | ||
| this.Id = Guid.NewGuid(); | ||
| IsFilter = isFilter; | ||
| Id = Guid.NewGuid(); | ||
| } | ||
|
|
||
| internal CompiledScriptBlockData(string scriptText, bool isProductCode) | ||
| { | ||
| _isProductCode = isProductCode; | ||
| _scriptText = scriptText; | ||
| this.Id = Guid.NewGuid(); | ||
| Id = Guid.NewGuid(); | ||
| } | ||
|
|
||
| internal bool Compile(bool optimized) | ||
|
|
@@ -78,6 +106,11 @@ internal bool Compile(bool optimized) | |
| CompileOptimized(); | ||
| } | ||
|
|
||
| if (ProfilerEventSource.LogInstance.IsEnabled()) | ||
| { | ||
| RegisterCompiledScriptBlockData(); | ||
| } | ||
|
|
||
| return optimized; | ||
| } | ||
|
|
||
|
|
@@ -131,6 +164,11 @@ private void InitializeMetadata() | |
|
|
||
| private void CompileUnoptimized() | ||
| { | ||
| if (_compiledUnoptimized) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| lock (this) | ||
| { | ||
| if (_compiledUnoptimized) | ||
|
|
@@ -146,6 +184,11 @@ private void CompileUnoptimized() | |
|
|
||
| private void CompileOptimized() | ||
| { | ||
| if (_compiledOptimized) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| lock (this) | ||
| { | ||
| if (_compiledOptimized) | ||
|
|
@@ -548,7 +591,7 @@ internal ScriptBlock(IParameterMetadataProvider ast, bool isFilter) | |
| { | ||
| } | ||
|
|
||
| private ScriptBlock(CompiledScriptBlockData scriptBlockData) | ||
| internal ScriptBlock(CompiledScriptBlockData scriptBlockData) | ||
| { | ||
| _scriptBlockData = scriptBlockData; | ||
|
|
||
|
|
@@ -576,24 +619,25 @@ protected ScriptBlock(SerializationInfo info, StreamingContext context) | |
| { | ||
| } | ||
|
|
||
| private static readonly ConcurrentDictionary<Tuple<string, string>, ScriptBlock> s_cachedScripts = | ||
| new ConcurrentDictionary<Tuple<string, string>, ScriptBlock>(); | ||
| private static readonly ConcurrentDictionary<Tuple<string, string>, CompiledScriptBlockData> s_cachedScripts = | ||
| new ConcurrentDictionary<Tuple<string, string>, CompiledScriptBlockData>(); | ||
|
|
||
| internal static ICollection<CompiledScriptBlockData> GetCachedCompiledScriptBlockData() | ||
| { | ||
| return s_cachedScripts.Values; | ||
| } | ||
|
|
||
| internal static ScriptBlock TryGetCachedScriptBlock(string fileName, string fileContents) | ||
| internal static CompiledScriptBlockData TryGetCachedCompiledScriptBlock(string fileName, string fileContents) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about keeping the method signature unchanged, but only changing the cache to hold |
||
| { | ||
| if (InternalTestHooks.IgnoreScriptBlockCache) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| ScriptBlock scriptBlock; | ||
| var key = Tuple.Create(fileName, fileContents); | ||
| if (s_cachedScripts.TryGetValue(key, out scriptBlock)) | ||
| if (s_cachedScripts.TryGetValue(key, out var compiledScriptBlockData)) | ||
| { | ||
| Diagnostics.Assert( | ||
| scriptBlock.SessionStateInternal == null, | ||
| "A cached scriptblock should not have it's session state bound, that causes a memory leak."); | ||
| return scriptBlock.Clone(); | ||
| return compiledScriptBlockData; | ||
| } | ||
|
|
||
| return null; | ||
|
|
@@ -605,7 +649,7 @@ private static bool IsDynamicKeyword(Ast ast) | |
| private static bool IsUsingTypes(Ast ast) | ||
| => ast is UsingStatementAst cmdAst && cmdAst.IsUsingModuleOrAssembly(); | ||
|
|
||
| internal static void CacheScriptBlock(ScriptBlock scriptBlock, string fileName, string fileContents) | ||
| internal static void CacheCompiledScriptBlock(ScriptBlock scriptBlock, string fileName, string fileContents) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, better make the signature unchanged. What to actually cache is an implementation detail. |
||
| { | ||
| if (InternalTestHooks.IgnoreScriptBlockCache) | ||
| { | ||
|
|
@@ -632,26 +676,18 @@ internal static void CacheScriptBlock(ScriptBlock scriptBlock, string fileName, | |
| } | ||
|
|
||
| var key = Tuple.Create(fileName, fileContents); | ||
| s_cachedScripts.TryAdd(key, scriptBlock); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Clears the cached scriptblocks. | ||
| /// </summary> | ||
| internal static void ClearScriptBlockCache() | ||
| { | ||
| s_cachedScripts.Clear(); | ||
| s_cachedScripts.TryAdd(key, scriptBlock.ScriptBlockData); | ||
daxian-dbw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| internal static readonly ScriptBlock EmptyScriptBlock = | ||
| ScriptBlock.CreateDelayParsedScriptBlock(string.Empty, isProductCode: true); | ||
|
|
||
| internal static ScriptBlock Create(Parser parser, string fileName, string fileContents) | ||
| { | ||
| var scriptBlock = TryGetCachedScriptBlock(fileName, fileContents); | ||
| if (scriptBlock != null) | ||
| var compiledScriptBlockData = TryGetCachedCompiledScriptBlock(fileName, fileContents); | ||
| if (compiledScriptBlockData != null) | ||
| { | ||
| return scriptBlock; | ||
| return new ScriptBlock(compiledScriptBlockData); | ||
| } | ||
|
|
||
| var ast = parser.Parse(fileName, fileContents, null, out ParseError[] errors, ParseMode.Default); | ||
|
|
@@ -661,12 +697,9 @@ internal static ScriptBlock Create(Parser parser, string fileName, string fileCo | |
| } | ||
|
|
||
| var result = new ScriptBlock(ast, isFilter: false); | ||
| CacheScriptBlock(result, fileName, fileContents); | ||
| CacheCompiledScriptBlock(result, fileName, fileContents); | ||
|
|
||
| // The value returned will potentially be bound to a session state. We don't want | ||
| // the cached script block to end up being bound to any session state, so clone | ||
| // the return value to ensure the cached value has no session state. | ||
| return result.Clone(); | ||
| return result; | ||
| } | ||
|
|
||
| internal ScriptBlock Clone() => new ScriptBlock(_scriptBlockData); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid making changes to the call sites of
TryGetCachedScriptBlock. Whether it cachesScriptBlockorCompiledScriptBlockDatashould be an implementation detail toTryGetCachedScriptBlock.