Fixed Forum-8700(asl): Refactored stepping to increase performance (using StepOut for tracking again)

git-svn-id: svn://svn.sharpdevelop.net/sharpdevelop/trunk@1538 1ccf3a8d-04fe-1044-b7c0-cef0b8235c61

src/AddIns/Misc/Debugger/Debugger.Core/Project/Debugger.Core.csproj

@@ -377,6 +377,9 @@
     <Compile Include="Src\Wrappers\CorDebug\ICorDebugStepper.cs" />
     <Compile Include="Src\Wrappers\CorDebug\ICorDebugChainEnum.cs" />
     <Compile Include="Src\Wrappers\CorDebug\ICorDebugFrameEnum.cs" />
+    <Compile Include="Src\Threads\FrameID.cs" />
+    <Compile Include="Src\Wrappers\CorDebug\ICorDebugChain.cs" />
+    <Compile Include="Src\Wrappers\CorDebug\ICorDebugFrame.cs" />
   </ItemGroup>
   <ItemGroup>
     <Content Include="README.TXT" />
@@ -387,6 +390,8 @@
     <Folder Include="Src\Wrappers\CorSym" />
     <Folder Include="Src\Wrappers\CorSym\Autogenerated" />
     <Folder Include="Src\Wrappers\MetaData" />
+    <Folder Include="docs" />
+    <Content Include="docs\Stepping.txt" />
   </ItemGroup>
   <Import Project="$(MSBuildBinPath)\Microsoft.CSHARP.Targets" />
-</Project>
+</Project>

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Debugger/Internal/ManagedCallback.cs

@@ -122,10 +122,17 @@ namespace Debugger
 				if (debugger.SelectedThread.LastFunction.HasSymbols) {
 					ExitCallback_Paused();
 				} else {
-					// This should not happen with JMC enabled
-					debugger.TraceMessage(" - leaving code without symbols");
-					
-					ExitCallback_Continue();
+					// This can only happen when JMC is disabled (ie NET1.1 or StepOut)
+					if (stepper.Operation == Stepper.StepperOperation.StepOut) {
+						// Create new stepper and keep going
+						debugger.TraceMessage(" - stepping out of code without symbols at " + debugger.SelectedThread.LastFunction.ToString());
+						new Stepper(debugger.SelectedThread.LastFunction, "Stepper out of code without symbols").StepOut();
+						ExitCallback_Continue();
+					} else {
+						// NET1.1: There is extra step over stepper, just keep going
+						debugger.TraceMessage(" - leaving code without symbols");
+						ExitCallback_Continue();
+					}
 				}
 			} else {
 				ExitCallback_Continue();

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Threads/FrameID.cs

@@ -0,0 +1,59 @@
+// <file>
+//     <copyright see="prj:///doc/copyright.txt"/>
+//     <license see="prj:///doc/license.txt"/>
+//     <owner name="David Srbecký" email="dsrbecky@gmail.com"/>
+//     <version>$Revision: 1534 $</version>
+// </file>
+
+using System;
+using System.Collections.Generic;
+using System.Runtime.InteropServices;
+using System.Threading;
+
+using Debugger.Wrappers.CorDebug;
+
+namespace Debugger
+{
+	/// <summary>
+	/// Identifies frame on thread callstack
+	/// </summary>
+	struct FrameID {
+		uint chainIndex;
+		uint frameIndex;
+		
+		public uint ChainIndex {
+			get {
+				return chainIndex;
+			}
+		}
+		
+		public uint FrameIndex {
+			get {
+				return frameIndex;
+			}
+		}
+		
+		public FrameID(uint chainIndex, uint frameIndex)
+		{
+			this.chainIndex = chainIndex;
+			this.frameIndex = frameIndex;
+		}
+		
+		public override int GetHashCode()
+		{
+			return chainIndex.GetHashCode() ^ frameIndex.GetHashCode();
+		}
+		
+		public override bool Equals(object obj)
+		{
+			if (!(obj is FrameID)) return false; 
+			FrameID myFrameID = (FrameID)obj;
+			return this.chainIndex == myFrameID.chainIndex && this.frameIndex == myFrameID.frameIndex;
+		}
+		
+		public override string ToString()
+		{
+			return string.Format("{0},{1}", this.chainIndex, this.frameIndex);
+		}
+	}
+}

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Threads/Function.cs

@@ -29,8 +29,7 @@ namespace Debugger
 		
 		bool steppedOut = false;
 		Thread thread;
-		uint chainIndex;
-		uint frameIndex;
+		FrameID frameID;
 		
 		MethodProps methodProps;
 		
@@ -42,7 +41,7 @@ namespace Debugger
 
 		public string Name { 
 			get { 
-				return methodProps.Name; // + "(" + chainIndex.ToString() + ", " + frameIndex.ToString() + ")";
+				return methodProps.Name;
 			} 
 		}
 		
@@ -100,6 +99,7 @@ namespace Debugger
 		{
 			if (!steppedOut) {
 				steppedOut = true;
+				debugger.TraceMessage("Function " + this.ToString() + " expired");
 				if (Expired != null) {
 					Expired(this, e);
 				}
@@ -130,37 +130,35 @@ namespace Debugger
 			}
 		}
 		
-		internal Function(Thread thread, uint chainIndex, uint frameIndex, ICorDebugILFrame corILFrame)
+		internal Function(Thread thread, FrameID frameID, ICorDebugILFrame corILFrame)
 		{
 			this.debugger = thread.Debugger;
 			this.thread = thread;
-			this.chainIndex = chainIndex;
-			this.frameIndex = frameIndex;
+			this.frameID = frameID;
 			this.CorILFrame = corILFrame;
 			corFunction = corILFrame.Function;
 			module = debugger.GetModule(corFunction.Module);
 			
 			methodProps = module.MetaData.GetMethodProps(corFunction.Token);
 			
-			AddTrackingStepper();
-		}
-		
-		internal void AddTrackingStepper()
-		{
-			// Expiry the function when it is finished
+			// Force some callback when function steps out so that we can expire it
 			stepOutStepper = new Stepper(this, "Function Tracker");
 			stepOutStepper.StepOut();
 			stepOutStepper.PauseWhenComplete = false;
-			stepOutStepper.StepComplete += delegate {
-				OnExpired(EventArgs.Empty);
-			};
+			
+			debugger.TraceMessage("Function " + this.ToString() + " created");
+		}
+		
+		public override string ToString()
+		{
+			return methodProps.Name + "(" + frameID.ToString() + ")";
 		}
 		
 		internal ICorDebugILFrame CorILFrame {
 			get {
 				if (HasExpired) throw new DebuggerException("Function has expired");
 				if (corILFramePauseSession != debugger.PauseSession) {
-					CorILFrame = thread.GetFrameAt(chainIndex, frameIndex).As<ICorDebugILFrame>();
+					CorILFrame = thread.GetFrameAt(frameID).As<ICorDebugILFrame>();
 				}
 				return corILFrame;
 			}
@@ -213,8 +211,7 @@ namespace Debugger
 
 		public void StepOut()
 		{
-			Stepper stepper = new Stepper(this);
-			stepper.StepOut();
+			new Stepper(this, "Function step out").StepOut();
 			debugger.Continue();
 		}
 
@@ -231,19 +228,15 @@ namespace Debugger
 				throw new DebuggerException("Unable to step. Next statement not aviable");
 			}
 			
-			Stepper stepper;
-			
 			if (stepIn) {
-				stepper = new Stepper(this);
-				stepper.StepIn(nextSt.StepRanges);
+				new Stepper(this, "Function step in").StepIn(nextSt.StepRanges);
+				// Without JMC step in which ends in code without symblols is cotinued.
+				// The next step over ensures that we at least do step over.
+				new Stepper(this, "Safety step over").StepOver(nextSt.StepRanges);
+			} else {
+				new Stepper(this, "Function step over").StepOver(nextSt.StepRanges);
 			}
 			
-			// Without JMC step in which ends in code without symblols is cotinued.
-			// The next step over ensures that we at least do step over.
-			
-			stepper = new Stepper(this);
-			stepper.StepOver(nextSt.StepRanges);
-			
 			debugger.Continue();
 		}
 		

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Threads/Stepper.cs

@@ -56,6 +56,15 @@ namespace Debugger
 			}
 		}
 		
+		public bool JustMyCode {
+			set {
+				if (corStepper.Is<ICorDebugStepper2>()) { // Is the debuggee .NET 2.0?
+					corStepper.SetUnmappedStopMask(CorDebugUnmappedStop.STOP_NONE);
+					corStepper.CastTo<ICorDebugStepper2>().SetJMC(value ? 1 : 0);
+				}
+			}
+		}
+		
 		public Stepper(Function function, string name): this(function)
 		{
 			this.name = name;
@@ -67,11 +76,7 @@ namespace Debugger
 			
 			corStepper = function.CorILFrame.CreateStepper();
 			
-			// Turn on Just-My-Code
-			if (corStepper.Is<ICorDebugStepper2>()) { // Is the debuggee .NET 2.0?
-				corStepper.SetUnmappedStopMask(CorDebugUnmappedStop.STOP_NONE);
-				corStepper.CastTo<ICorDebugStepper2>().SetJMC(1 /* true */);
-			}
+			JustMyCode = true;
 			
 			function.Thread.Steppers.Add(this);
 		}
@@ -87,15 +92,11 @@ namespace Debugger
 			return this.corStepper == corStepper;
 		}
 		
-		// NOTE: corStepper.StepOut(); finishes when pevious frame is activated, not when function is exited
-		//       this is important for events with multiple handlers
-		// NOTE: StepRange callbacks go first (probably in order),
-		//       StepOut callback are called after that
 		public void StepOut()
 		{
 			operation = StepperOperation.StepOut;
-			// corStepper.StepOut(); // Don't! see note
-			corStepper.StepRange(false, new int[] {0, int.MaxValue});
+			JustMyCode = false; // Needed for multiple events. See docs\Stepping.txt
+			corStepper.StepOut();
 		}
 		
 		public void StepIn(int[] ranges)
@@ -112,7 +113,7 @@ namespace Debugger
 		
 		public override string ToString()
 		{
-			return string.Format("{0} in {1} pause={2} {3}", Operation, Function.Name, PauseWhenComplete, name);
+			return string.Format("{0} in {1} pause={2} \"{3}\"", Operation, Function.ToString(), PauseWhenComplete, name);
 		}
 	}
 }

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Threads/Thread.cs

@@ -179,81 +179,58 @@ namespace Debugger
 			get {
 				process.AssertPaused();
 				
-				ICorDebugChainEnum corChainEnum = corThread.EnumerateChains();
-				uint chainIndex = corChainEnum.Count;
-				foreach(ICorDebugChain corChain in corChainEnum.Enumerator) {
-					chainIndex--;
-					
+				foreach(ICorDebugChain corChain in corThread.EnumerateChains().Enumerator) {
 					if (corChain.IsManaged == 0) continue; // Only managed ones
-					
-					ICorDebugFrameEnum corFrameEnum = corChain.EnumerateFrames();
-					uint frameIndex = corFrameEnum.Count;
-					foreach(ICorDebugFrame corFrame in corFrameEnum.Enumerator) {
-						frameIndex--;
-						
+					foreach(ICorDebugFrame corFrame in corChain.EnumerateFrames().Enumerator) {
 						if (corFrame.Is<ICorDebugILFrame>()) {
-							Function function = GetFunctionFromCache(chainIndex, frameIndex, corFrame.As<ICorDebugILFrame>());
-							if (function != null) {
-								yield return function;
-							}
+							Function function;
+							try {
+								function = GetFunctionFromCache(new FrameID(corChain.Index, corFrame.Index), corFrame.As<ICorDebugILFrame>());
+							} catch (COMException) { // TODO
+								continue;
+							};
+							yield return function;
 						}
 					}
 				}
 			}
 		}
 		
-		Dictionary<uint, Chain> chainCache = new Dictionary<uint, Chain>();
+		Dictionary<FrameID, Function> functionCache = new Dictionary<FrameID, Function>();
 		
-		class Chain {
-			public Dictionary<uint, Function> Frames = new Dictionary<uint, Function>();
-		}
-		
-		Function GetFunctionFromCache(uint chainIndex, uint frameIndex, ICorDebugILFrame corFrame)
+		Function GetFunctionFromCache(FrameID frameID, ICorDebugILFrame corFrame)
 		{
-			try {
-				if (chainCache.ContainsKey(chainIndex) &&
-				    chainCache[chainIndex].Frames.ContainsKey(frameIndex) &&
-				    !chainCache[chainIndex].Frames[frameIndex].HasExpired) {
-					
-					Function function = chainCache[chainIndex].Frames[frameIndex];
-					function.CorILFrame = corFrame;
-					return function;
-				} else {
-					Function function = new Function(this, chainIndex, frameIndex, corFrame.CastTo<ICorDebugILFrame>());
-					if (!chainCache.ContainsKey(chainIndex)) chainCache[chainIndex] = new Chain();
-					chainCache[chainIndex].Frames[frameIndex] = function;
-					function.Expired += delegate { chainCache[chainIndex].Frames.Remove(frameIndex); };
-					return function;
-				}
-			} catch (COMException) { // TODO
-				return null;
-			};
+			Function function;
+			if (functionCache.TryGetValue(frameID, out function) && !function.HasExpired) {
+				function.CorILFrame = corFrame;
+				return function;
+			} else {
+				function = new Function(this, frameID, corFrame);
+				functionCache[frameID] = function;
+				return function;
+			}
 		}
 		
-		internal ICorDebugFrame GetFrameAt(uint chainIndex, uint frameIndex)
+		internal ICorDebugFrame GetFrameAt(FrameID frameID)
 		{
 			process.AssertPaused();
 			
 			ICorDebugChainEnum corChainEnum = corThread.EnumerateChains();
-			if (chainIndex >= corChainEnum.Count) throw new ArgumentException("Chain index too big", "chainIndex");
-			corChainEnum.Skip(corChainEnum.Count - chainIndex - 1);
+			if (frameID.ChainIndex >= corChainEnum.Count) throw new ArgumentException("Chain index too big", "chainIndex");
+			corChainEnum.Skip(corChainEnum.Count - frameID.ChainIndex - 1);
 			
 			ICorDebugChain corChain = corChainEnum.Next();
 			
 			if (corChain.IsManaged == 0) throw new ArgumentException("Chain is not managed", "chainIndex");
 			
 			ICorDebugFrameEnum corFrameEnum = corChain.EnumerateFrames();
-			if (frameIndex >= corFrameEnum.Count) throw new ArgumentException("Frame index too big", "frameIndex");
-			corFrameEnum.Skip(corFrameEnum.Count - frameIndex - 1);
+			if (frameID.FrameIndex >= corFrameEnum.Count) throw new ArgumentException("Frame index too big", "frameIndex");
+			corFrameEnum.Skip(corFrameEnum.Count - frameID.FrameIndex - 1);
 			
 			return corFrameEnum.Next();
 		}
 		
-		// NOTE: During evlulation some chains may be temporaly removed
-		// NOTE: When two events are invoked, step outs ocurr at once when all is done
-		// NOTE: Step out works properly for exceptions
-		// NOTE: Step over works properly for exceptions
-		// NOTE: Evaluation kills stepper overs on active frame
+		// See docs\Stepping.txt
 		internal void CheckExpirationOfFunctions()
 		{
 			if (debugger.Evaluating) return;
@@ -266,31 +243,29 @@ namespace Debugger
 			
 			ICorDebugFrame lastFrame = corFrameEnum.Next();
 			
-			List<Function> expiredFunctions = new List<Function>();
-			
-			foreach(KeyValuePair<uint, Chain> chain in chainCache) {
-				if (chain.Key < maxChainIndex) continue;
-				foreach(KeyValuePair<uint, Function> func in chain.Value.Frames) {
-					if (chain.Key == maxChainIndex && func.Key <= maxFrameIndex) continue;
-					expiredFunctions.Add(func.Value);
-				}
-			}
-			
-			// Check the token of the last function
-			// TODO: Investigate: this should not happen (test case: event with two handlers)
-			if (lastFrame != null &&
-			    chainCache.ContainsKey(maxChainIndex) &&
-			    chainCache[maxChainIndex].Frames.ContainsKey(maxFrameIndex)) {
+			// Check the token of the current function - function can change if there are multiple handlers for an event
+			Function function;
+			if (lastFrame != null && 
+			    functionCache.TryGetValue(new FrameID(maxChainIndex, maxFrameIndex), out function) &&
+			    function.Token != lastFrame.FunctionToken) {
 				
-				Function cachedFunction = chainCache[maxChainIndex].Frames[maxFrameIndex];
-				if (cachedFunction.Token != lastFrame.FunctionToken) {
-					expiredFunctions.Add(cachedFunction);
+				functionCache.Remove(new FrameID(maxChainIndex, maxFrameIndex));
+				function.OnExpired(EventArgs.Empty);
+			}
+
+			// Expire all functions behind the current maximum
+			// Multiple functions can expire at once (test case: Step out of Button1Click in simple winforms application)
+			List<KeyValuePair<FrameID, Function>> toBeRemoved = new List<KeyValuePair<FrameID, Function>>();
+			foreach(KeyValuePair<FrameID, Function> kvp in functionCache) {
+				if ((kvp.Key.ChainIndex > maxChainIndex) ||
+				    (kvp.Key.ChainIndex == maxChainIndex && kvp.Key.FrameIndex > maxFrameIndex)) {
+					
+					toBeRemoved.Add(kvp);
 				}
 			}
-			
-			foreach(Function f in expiredFunctions) {
-				debugger.TraceMessage("Function " + f.Name + " expired. (check)");
-				f.OnExpired(EventArgs.Empty);
+			foreach(KeyValuePair<FrameID, Function> kvp in toBeRemoved){
+				functionCache.Remove(kvp.Key);
+				kvp.Value.OnExpired(EventArgs.Empty);
 			}
 		}
 		

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Variables/Evals/Eval.cs

@@ -154,14 +154,6 @@ namespace Debugger
 			
 			OnEvalStarted(new EvalEventArgs(this));
 			
-			// Stepper needs to be reset after evaluation
-			Function lastFunction = targetThread.LastFunction;
-			if (lastFunction != null) { // TODO: Investigate
-				EvalComplete += delegate {
-					lastFunction.AddTrackingStepper();
-				};
-			}
-			
 			evalState = EvalState.Evaluating;
 			return true;
 		}

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Wrappers/CorDebug/ICorDebugChain.cs

@@ -0,0 +1,27 @@
+// <file>
+//     <copyright see="prj:///doc/copyright.txt"/>
+//     <license see="prj:///doc/license.txt"/>
+//     <owner name="David Srbecký" email="dsrbecky@gmail.com"/>
+//     <version>$Revision$</version>
+// </file>
+
+namespace Debugger.Wrappers.CorDebug
+{
+	using System;
+	using System.Collections.Generic;
+	
+	
+	public partial class ICorDebugChain
+	{
+		uint index;
+		
+		public uint Index {
+			get {
+				return index;
+			}
+			set {
+				index = value;
+			}
+		}
+	}
+}

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Wrappers/CorDebug/ICorDebugChainEnum.cs

@@ -15,9 +15,12 @@ namespace Debugger.Wrappers.CorDebug
 	{
 		public IEnumerable<ICorDebugChain> Enumerator {
 			get {
+				Reset();
+				uint index = this.Count - 1;
 				while (true) {
 					ICorDebugChain corChain = Next();
 					if (corChain != null) {
+						corChain.Index = index--;
 						yield return corChain;
 					} else {
 						break;

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Wrappers/CorDebug/ICorDebugFrame.cs

@@ -0,0 +1,27 @@
+// <file>
+//     <copyright see="prj:///doc/copyright.txt"/>
+//     <license see="prj:///doc/license.txt"/>
+//     <owner name="David Srbecký" email="dsrbecky@gmail.com"/>
+//     <version>$Revision$</version>
+// </file>
+
+namespace Debugger.Wrappers.CorDebug
+{
+	using System;
+	using System.Collections.Generic;
+	
+	
+	public partial class ICorDebugFrame
+	{
+		uint index;
+		
+		public uint Index {
+			get {
+				return index;
+			}
+			set {
+				index = value;
+			}
+		}
+	}
+}

src/AddIns/Misc/Debugger/Debugger.Core/Project/Src/Wrappers/CorDebug/ICorDebugFrameEnum.cs

@@ -15,9 +15,12 @@ namespace Debugger.Wrappers.CorDebug
 	{
 		public IEnumerable<ICorDebugFrame> Enumerator {
 			get {
+				Reset();
+				uint index = this.Count - 1;
 				while (true) {
 					ICorDebugFrame corFrame = Next();
 					if (corFrame != null) {
+						corFrame.Index = index--;
 						yield return corFrame;
 					} else {
 						break;

src/AddIns/Misc/Debugger/Debugger.Core/Project/docs/Stepping.txt

@@ -0,0 +1,7 @@
+== Notes ==
+- During evaluation some chains may be temporarly removed
+- When two events are invoked and JMC is active, step out skips the second function
+- Step out and step over works properly for exceptions
+- Evaluation kills stepper overs on active frame
+- StepRange callbacks go first (probably in order), StepOut callback are called after that
+- StepRange is much slower then StepOut