Skip to content

Commit d8decdc

Browse files
nohwndnohwnd
authored
Fix slow execution when many breakpoints are used (PowerShell#14953)
1 parent e1aacd7 commit d8decdc

3 files changed

Lines changed: 76 additions & 44 deletions

File tree

‎src/System.Management.Automation/engine/debugger/Breakpoint.cs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -482,9 +482,6 @@ internal bool TrySetBreakpoint(string scriptFile, FunctionContext functionContex
482482
{
483483
Diagnostics.Assert(SequencePointIndex == -1, "shouldn't be trying to set on a pending breakpoint");
484484

485-
if (!scriptFile.Equals(this.Script, StringComparison.OrdinalIgnoreCase))
486-
return false;
487-
488485
// A quick check to see if the breakpoint is within the scriptblock.
489486
bool couldBeInNestedScriptBlock;
490487
var scriptBlock = functionContext._scriptBlock;
@@ -595,11 +592,11 @@ internal override bool RemoveSelf(ScriptDebugger debugger)
595592
var boundBreakPoints = debugger.GetBoundBreakpoints(this.SequencePoints);
596593
if (boundBreakPoints != null)
597594
{
598-
Diagnostics.Assert(boundBreakPoints.Contains(this),
595+
Diagnostics.Assert(boundBreakPoints[this.SequencePointIndex].Contains(this),
599596
"If we set _scriptBlock, we should have also added the breakpoint to the bound breakpoint list");
600-
boundBreakPoints.Remove(this);
597+
boundBreakPoints[this.SequencePointIndex].Remove(this);
601598

602-
if (boundBreakPoints.All(breakpoint => breakpoint.SequencePointIndex != this.SequencePointIndex))
599+
if (boundBreakPoints[this.SequencePointIndex].All(breakpoint => breakpoint.SequencePointIndex != this.SequencePointIndex))
603600
{
604601
// No other line breakpoints are at the same sequence point, so disable the breakpoint so
605602
// we don't go looking for breakpoints the next time we hit the sequence point.

‎src/System.Management.Automation/engine/debugger/debugger.cs‎

Lines changed: 72 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,8 @@ internal ScriptDebugger(ExecutionContext context)
981981
_context = context;
982982
_inBreakpoint = false;
983983
_idToBreakpoint = new ConcurrentDictionary<int, Breakpoint>();
984-
_pendingBreakpoints = new ConcurrentDictionary<int, LineBreakpoint>();
984+
// The string key is function context file path. The int key is sequencePoint index.
985+
_pendingBreakpoints = new ConcurrentDictionary<string, ConcurrentDictionary<int, LineBreakpoint>>();
985986
_boundBreakpoints = new ConcurrentDictionary<string, Tuple<WeakReference, ConcurrentDictionary<int, LineBreakpoint>>>(StringComparer.OrdinalIgnoreCase);
986987
_commandBreakpoints = new ConcurrentDictionary<int, CommandBreakpoint>();
987988
_variableBreakpoints = new ConcurrentDictionary<string, ConcurrentDictionary<int, VariableBreakpoint>>(StringComparer.OrdinalIgnoreCase);
@@ -1184,7 +1185,7 @@ internal void EnterScriptFunction(FunctionContext functionContext)
11841185
private void SetupBreakpoints(FunctionContext functionContext)
11851186
{
11861187
var scriptDebugData = _mapScriptToBreakpoints.GetValue(functionContext._sequencePoints,
1187-
_ => Tuple.Create(new List<LineBreakpoint>(),
1188+
_ => Tuple.Create(new Dictionary<int, List<LineBreakpoint>>(),
11881189
new BitArray(functionContext._sequencePoints.Length)));
11891190
functionContext._boundBreakpoints = scriptDebugData.Item1;
11901191
functionContext._breakPoints = scriptDebugData.Item2;
@@ -1264,10 +1265,19 @@ private CommandBreakpoint AddCommandBreakpoint(CommandBreakpoint breakpoint)
12641265
private LineBreakpoint AddLineBreakpoint(LineBreakpoint breakpoint)
12651266
{
12661267
AddBreakpointCommon(breakpoint);
1267-
_pendingBreakpoints[breakpoint.Id] = breakpoint;
1268+
AddPendingBreakpoint(breakpoint);
1269+
12681270
return breakpoint;
12691271
}
12701272

1273+
private void AddPendingBreakpoint(LineBreakpoint breakpoint)
1274+
{
1275+
_pendingBreakpoints.AddOrUpdate(
1276+
breakpoint.Script,
1277+
new ConcurrentDictionary<int, LineBreakpoint> { [breakpoint.Id] = breakpoint },
1278+
(_, dictionary) => { dictionary.TryAdd(breakpoint.Id, breakpoint); return dictionary; });
1279+
}
1280+
12711281
private void AddNewBreakpoint(Breakpoint breakpoint)
12721282
{
12731283
LineBreakpoint lineBreakpoint = breakpoint as LineBreakpoint;
@@ -1320,13 +1330,9 @@ private void UpdateBreakpoints(FunctionContext functionContext)
13201330
return;
13211331
}
13221332

1323-
foreach ((int breakpointId, LineBreakpoint item) in _pendingBreakpoints)
1333+
if (_pendingBreakpoints.TryGetValue(functionContext._file, out var dictionary) && !dictionary.IsEmpty)
13241334
{
1325-
if (item.IsScriptBreakpoint && item.Script.Equals(functionContext._file, StringComparison.OrdinalIgnoreCase))
1326-
{
1327-
SetPendingBreakpoints(functionContext);
1328-
break;
1329-
}
1335+
SetPendingBreakpoints(functionContext);
13301336
}
13311337
}
13321338
}
@@ -1352,7 +1358,11 @@ internal bool RemoveCommandBreakpoint(CommandBreakpoint breakpoint) =>
13521358

13531359
internal bool RemoveLineBreakpoint(LineBreakpoint breakpoint)
13541360
{
1355-
bool removed = _pendingBreakpoints.Remove(breakpoint.Id, out _);
1361+
bool removed = false;
1362+
if (_pendingBreakpoints.TryGetValue(breakpoint.Script, out var dictionary))
1363+
{
1364+
removed = dictionary.Remove(breakpoint.Id, out _);
1365+
}
13561366

13571367
Tuple<WeakReference, ConcurrentDictionary<int, LineBreakpoint>> value;
13581368
if (_boundBreakpoints.TryGetValue(breakpoint.Script, out value))
@@ -1370,8 +1380,8 @@ internal bool RemoveLineBreakpoint(LineBreakpoint breakpoint)
13701380
// The bit array is used to detect if a breakpoint is set or not for a given scriptblock. This bit array
13711381
// is checked when hitting sequence points. Enabling/disabling a line breakpoint is as simple as flipping
13721382
// the bit.
1373-
private readonly ConditionalWeakTable<IScriptExtent[], Tuple<List<LineBreakpoint>, BitArray>> _mapScriptToBreakpoints =
1374-
new ConditionalWeakTable<IScriptExtent[], Tuple<List<LineBreakpoint>, BitArray>>();
1383+
private readonly ConditionalWeakTable<IScriptExtent[], Tuple<Dictionary<int, List<LineBreakpoint>>, BitArray>> _mapScriptToBreakpoints =
1384+
new ConditionalWeakTable<IScriptExtent[], Tuple<Dictionary<int, List<LineBreakpoint>>, BitArray>>();
13751385

13761386
/// <summary>
13771387
/// Checks for command breakpoints.
@@ -1472,9 +1482,9 @@ internal void TriggerVariableBreakpoints(List<VariableBreakpoint> breakpoints)
14721482

14731483
// Return the line breakpoints bound in a specific script block (used when a sequence point
14741484
// is hit, to find which breakpoints are set on that sequence point.)
1475-
internal List<LineBreakpoint> GetBoundBreakpoints(IScriptExtent[] sequencePoints)
1485+
internal Dictionary<int, List<LineBreakpoint>> GetBoundBreakpoints(IScriptExtent[] sequencePoints)
14761486
{
1477-
Tuple<List<LineBreakpoint>, BitArray> tuple;
1487+
Tuple<Dictionary<int, List<LineBreakpoint>>, BitArray> tuple;
14781488
if (_mapScriptToBreakpoints.TryGetValue(sequencePoints, out tuple))
14791489
{
14801490
return tuple.Item1;
@@ -1560,16 +1570,25 @@ internal void OnSequencePointHit(FunctionContext functionContext)
15601570
{
15611571
if (functionContext._breakPoints[functionContext._currentSequencePointIndex])
15621572
{
1563-
var breakpoints = (from breakpoint in functionContext._boundBreakpoints
1564-
where
1565-
breakpoint.SequencePointIndex == functionContext._currentSequencePointIndex &&
1566-
breakpoint.Enabled
1567-
select breakpoint).ToList<Breakpoint>();
1568-
1569-
breakpoints = TriggerBreakpoints(breakpoints);
1570-
if (breakpoints.Count > 0)
1573+
if (functionContext._boundBreakpoints.TryGetValue(functionContext._currentSequencePointIndex, out var sequencePointBreakpoints))
15711574
{
1572-
StopOnSequencePoint(functionContext, breakpoints);
1575+
var enabledBreakpoints = new List<Breakpoint>();
1576+
foreach (Breakpoint breakpoint in sequencePointBreakpoints)
1577+
{
1578+
if (breakpoint.Enabled)
1579+
{
1580+
enabledBreakpoints.Add(breakpoint);
1581+
}
1582+
}
1583+
1584+
if (enabledBreakpoints.Count > 0)
1585+
{
1586+
enabledBreakpoints = TriggerBreakpoints(enabledBreakpoints);
1587+
if (enabledBreakpoints.Count > 0)
1588+
{
1589+
StopOnSequencePoint(functionContext, enabledBreakpoints);
1590+
}
1591+
}
15731592
}
15741593
}
15751594
}
@@ -1683,7 +1702,7 @@ internal void Clear()
16831702
}
16841703

16851704
private readonly ExecutionContext _context;
1686-
private ConcurrentDictionary<int, LineBreakpoint> _pendingBreakpoints;
1705+
private readonly ConcurrentDictionary<string, ConcurrentDictionary<int, LineBreakpoint>> _pendingBreakpoints;
16871706
private readonly ConcurrentDictionary<string, Tuple<WeakReference, ConcurrentDictionary<int, LineBreakpoint>>> _boundBreakpoints;
16881707
private readonly ConcurrentDictionary<int, CommandBreakpoint> _commandBreakpoints;
16891708
private readonly ConcurrentDictionary<string, ConcurrentDictionary<int, VariableBreakpoint>> _variableBreakpoints;
@@ -1991,48 +2010,53 @@ private void UnbindBoundBreakpoints(List<LineBreakpoint> boundBreakpoints)
19912010
foreach (var breakpoint in boundBreakpoints)
19922011
{
19932012
// Also remove unbound breakpoints from the script to breakpoint map.
1994-
Tuple<List<LineBreakpoint>, BitArray> lineBreakTuple;
2013+
Tuple<Dictionary<int, List<LineBreakpoint>>, BitArray> lineBreakTuple;
19952014
if (_mapScriptToBreakpoints.TryGetValue(breakpoint.SequencePoints, out lineBreakTuple))
19962015
{
1997-
lineBreakTuple.Item1.Remove(breakpoint);
2016+
if (lineBreakTuple.Item1.TryGetValue(breakpoint.SequencePointIndex, out var lineBreakList))
2017+
{
2018+
lineBreakList.Remove(breakpoint);
2019+
}
19982020
}
19992021

20002022
breakpoint.SequencePoints = null;
20012023
breakpoint.SequencePointIndex = -1;
20022024
breakpoint.BreakpointBitArray = null;
2003-
_pendingBreakpoints[breakpoint.Id] = breakpoint;
2025+
2026+
AddPendingBreakpoint(breakpoint);
20042027
}
20052028

20062029
boundBreakpoints.Clear();
20072030
}
20082031

20092032
private void SetPendingBreakpoints(FunctionContext functionContext)
20102033
{
2011-
if (_pendingBreakpoints.IsEmpty)
2012-
return;
2013-
2014-
var newPendingBreakpoints = new Dictionary<int, LineBreakpoint>();
20152034
var currentScriptFile = functionContext._file;
20162035

20172036
// If we're not in a file, we can't have any line breakpoints.
20182037
if (currentScriptFile == null)
20192038
return;
20202039

2040+
if (!_pendingBreakpoints.TryGetValue(currentScriptFile, out var breakpoints) || breakpoints.IsEmpty)
2041+
{
2042+
return;
2043+
}
2044+
20212045
// Normally we register a script file when the script is run or the module is imported,
20222046
// but if there weren't any breakpoints when the script was run and the script was dotted,
20232047
// we will end up here with pending breakpoints, but we won't have cached the list of
20242048
// breakpoints in the script.
20252049
RegisterScriptFile(currentScriptFile, functionContext.CurrentPosition.StartScriptPosition.GetFullScript());
20262050

2027-
Tuple<List<LineBreakpoint>, BitArray> tuple;
2051+
Tuple<Dictionary<int, List<LineBreakpoint>>, BitArray> tuple;
20282052
if (!_mapScriptToBreakpoints.TryGetValue(functionContext._sequencePoints, out tuple))
20292053
{
20302054
Diagnostics.Assert(false, "If the script block is still alive, the entry should not be collected.");
20312055
}
20322056

20332057
Diagnostics.Assert(tuple.Item1 == functionContext._boundBreakpoints, "What's up?");
20342058

2035-
foreach ((int breakpointId, LineBreakpoint breakpoint) in _pendingBreakpoints)
2059+
foreach ((int breakpointId, LineBreakpoint breakpoint) in breakpoints)
20362060
{
20372061
bool bound = false;
20382062
if (breakpoint.TrySetBreakpoint(currentScriptFile, functionContext))
@@ -2043,21 +2067,32 @@ private void SetPendingBreakpoints(FunctionContext functionContext)
20432067
}
20442068

20452069
bound = true;
2046-
tuple.Item1.Add(breakpoint);
20472070

2071+
if (tuple.Item1.TryGetValue(breakpoint.SequencePointIndex, out var list))
2072+
{
2073+
list.Add(breakpoint);
2074+
}
2075+
else
2076+
{
2077+
tuple.Item1.Add(breakpoint.SequencePointIndex, new List<LineBreakpoint> { breakpoint });
2078+
}
2079+
20482080
// We need to keep track of any breakpoints that are bound in each script because they may
20492081
// need to be rebound if the script changes.
20502082
var boundBreakpoints = _boundBreakpoints[currentScriptFile].Item2;
20512083
boundBreakpoints[breakpoint.Id] = breakpoint;
20522084
}
20532085

2054-
if (!bound)
2086+
if (bound)
20552087
{
2056-
newPendingBreakpoints.Add(breakpoint.Id, breakpoint);
2088+
breakpoints.TryRemove(breakpointId, out _);
20572089
}
20582090
}
20592091

2060-
_pendingBreakpoints = new ConcurrentDictionary<int, LineBreakpoint>(newPendingBreakpoints);
2092+
// Here could check if all breakpoints for the current functionContext were bound, but because there is no atomic
2093+
// api for conditional removal we either need to lock, or do some trickery that has possibility of race conditions.
2094+
// Instead we keep the item in the dictionary with 0 breakpoint count. This should not be a big issue,
2095+
// because it is single entry per file that had breakpoints, so there won't be thousands of files in a session.
20612096
}
20622097

20632098
private void StopOnSequencePoint(FunctionContext functionContext, List<Breakpoint> breakpoints)

‎src/System.Management.Automation/engine/parser/Compiler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -782,7 +782,7 @@ internal class FunctionContext
782782
internal ExecutionContext _executionContext;
783783
internal Pipe _outputPipe;
784784
internal BitArray _breakPoints;
785-
internal List<LineBreakpoint> _boundBreakpoints;
785+
internal Dictionary<int, List<LineBreakpoint>> _boundBreakpoints;
786786
internal int _currentSequencePointIndex;
787787
internal MutableTuple _localsTuple;
788788
internal List<Tuple<Type[], Action<FunctionContext>[], Type[]>> _traps = new List<Tuple<Type[], Action<FunctionContext>[], Type[]>>();

0 commit comments

Comments
 (0)