[gh-4] Added optimization where breakpoint hit events are not subscri…

…bed to if actionable breakpoints exist.
ashmind · Feb 11, 2015 · c0601fa · c0601fa
1 parent f3c45d2
commit c0601fa
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 26 deletions.
diff --git a/ExceptionBreaker/Breakpoints/BreakpointExceptionsController.cs b/ExceptionBreaker/Breakpoints/BreakpointExceptionsController.cs
@@ -32,7 +32,7 @@ private void command_BeforeQueryStatus(object sender, EventArgs e) {
 
                 var extraData = _extraDataStore.GetData(breakpoint);
 
-                var @checked = extraData.ExceptionBreakChange.Value != ExceptionBreakChange.NoChange;
+                var @checked = extraData.ExceptionBreakChange != ExceptionBreakChange.NoChange;
                 if (@checked == _command.Checked)
                     return;
 
@@ -61,8 +61,8 @@ private void RequestAndUpdateExceptionSettings(Breakpoint2 breakpoint) {
             var extraData = _extraDataStore.GetData(breakpoint);
             var dialog = new BreakpointExceptionsDialog {
                 ViewModel = new BreakpointExceptionsViewModel {
-                    ShouldChange = { Value = extraData.ExceptionBreakChange.Value != ExceptionBreakChange.NoChange },
-                    Change = extraData.ExceptionBreakChange.Value,
+                    ShouldChange = { Value = extraData.ExceptionBreakChange != ExceptionBreakChange.NoChange },
+                    Change = extraData.ExceptionBreakChange,
                     ContinueExecution = !breakpoint.BreakWhenHit
                 }
             };
@@ -74,7 +74,7 @@ private void RequestAndUpdateExceptionSettings(Breakpoint2 breakpoint) {
 
             var change = dialog.ViewModel.ShouldChange.Value ? dialog.ViewModel.Change : ExceptionBreakChange.NoChange;
             _logger.WriteLine("Updating breakpoint settings: continue execution = {0}, change = {1}.", dialog.ViewModel.ContinueExecution, change);
-            extraData.ExceptionBreakChange.Value = change;
+            extraData.ExceptionBreakChange = change;
 
             SetBreakWhenHit(breakpoint, !dialog.ViewModel.ContinueExecution);
             _extraDataStore.NotifyDataChanged(breakpoint);

diff --git a/ExceptionBreaker/Breakpoints/BreakpointExtraData.cs b/ExceptionBreaker/Breakpoints/BreakpointExtraData.cs
@@ -1,15 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Text;
-using ExceptionBreaker.Core.Observable;
 
 namespace ExceptionBreaker.Breakpoints {
     public class BreakpointExtraData {
-        public BreakpointExtraData() {
-            ExceptionBreakChange = new ObservableValue<ExceptionBreakChange>();
-        }
-
-        public ObservableValue<ExceptionBreakChange> ExceptionBreakChange { get; private set; }
+        public ExceptionBreakChange ExceptionBreakChange { get; set; }
     }
 }
diff --git a/ExceptionBreaker/Breakpoints/BreakpointExtraDataChangedEventArgs.cs b/ExceptionBreaker/Breakpoints/BreakpointExtraDataChangedEventArgs.cs
@@ -1,12 +1,15 @@
 using System;
 using EnvDTE80;
+using JetBrains.Annotations;
 
 namespace ExceptionBreaker.Breakpoints {
     public class BreakpointExtraDataChangedEventArgs : EventArgs {
-        public BreakpointExtraDataChangedEventArgs(Breakpoint2 breakpoint) {
+        public BreakpointExtraDataChangedEventArgs(Breakpoint2 breakpoint, BreakpointExtraData data) {
             Breakpoint = breakpoint;
+            Data = data;
         }
 
-        public Breakpoint2 Breakpoint { get; private set; }
+        [NotNull] public Breakpoint2 Breakpoint   { get; private set; }
+        [NotNull] public BreakpointExtraData Data { get; private set; }
     }
 }
diff --git a/ExceptionBreaker/Breakpoints/BreakpointExtraDataStore.cs b/ExceptionBreaker/Breakpoints/BreakpointExtraDataStore.cs
@@ -14,13 +14,16 @@
 namespace ExceptionBreaker.Breakpoints {
     [Export]
     public class BreakpointExtraDataStore : ISolutionDataPersister {
+        private static readonly BreakpointExtraData EmptyData = new BreakpointExtraData();
+
+        public event EventHandler DataLoaded = delegate { };
+        public event EventHandler<BreakpointExtraDataChangedEventArgs> DataChanged = delegate { };
+
         private readonly BreakpointKeyProvider _keyProvider;
         private readonly JsonSerializer _jsonSerializer;
         private readonly IDiagnosticLogger _logger;
         private readonly ConcurrentDictionary<string, BreakpointExtraData> _store = new ConcurrentDictionary<string, BreakpointExtraData>(StringComparer.InvariantCultureIgnoreCase);
 
-        public event EventHandler<BreakpointExtraDataChangedEventArgs> DataChanged = delegate {};
-
         [ImportingConstructor]
         public BreakpointExtraDataStore(
             BreakpointKeyProvider keyProvider,
@@ -41,8 +44,10 @@ IDiagnosticLogger logger
                 foreach (Breakpoint2 old in e.OldItems) {
                     BreakpointExtraData data;
                     var removed = _store.TryRemove(_keyProvider.GetKey(old), out data);
-                    if (removed)
-                        DataChanged(this, new BreakpointExtraDataChangedEventArgs(old));
+                    if (removed) {
+                        data.ExceptionBreakChange = ExceptionBreakChange.NoChange;
+                        DataChanged(this, new BreakpointExtraDataChangedEventArgs(old, data));
+                    }
                 }
             };
         }
@@ -67,7 +72,11 @@ private BreakpointExtraData GetData([NotNull] string key) {
         }
 
         public void NotifyDataChanged([NotNull] Breakpoint2 breakpoint) {
-            DataChanged(this, new BreakpointExtraDataChangedEventArgs(breakpoint));
+            DataChanged(this, new BreakpointExtraDataChangedEventArgs(breakpoint, GetData(breakpoint)));
+        }
+
+        public IEnumerable<BreakpointExtraData> GetAllCurrentData() {
+            return _store.Values;
         }
 
         string ISolutionDataPersister.Key {
@@ -94,6 +103,8 @@ void ISolutionDataPersister.LoadFrom(Stream stream) {
                     _logger.WriteLine("  Loaded '{0}': change = {1}.", pair.Key, pair.Value.ExceptionBreakChange);
                 }
             }
+
+            DataLoaded(this, EventArgs.Empty);
         }
 
         #region BreakpointExtraDataSerialized Class
@@ -104,12 +115,12 @@ public BreakpointExtraDataSerializable() {
 
             public BreakpointExtraDataSerializable(BreakpointExtraData data) {
                 Version = 1;
-                ExceptionBreakChange = data.ExceptionBreakChange.Value;
+                ExceptionBreakChange = data.ExceptionBreakChange;
             }
 
             public BreakpointExtraData ToData() {
                 return new BreakpointExtraData {
-                    ExceptionBreakChange = {Value = ExceptionBreakChange}
+                    ExceptionBreakChange = ExceptionBreakChange
                 };
             }
 

diff --git a/ExceptionBreaker/Breakpoints/BreakpointHitHandler.cs b/ExceptionBreaker/Breakpoints/BreakpointHitHandler.cs
@@ -12,14 +12,58 @@ public class BreakpointHitHandler : IDebugEventCallback2, IDisposable {
         private readonly BreakpointExtraDataStore _extraDataStore;
         private readonly ExceptionBreakManager _breakManager;
         private readonly IDiagnosticLogger _logger;
+        private readonly ISet<BreakpointExtraData> _actionableExtraData = new HashSet<BreakpointExtraData>();
+        private bool _eventsAdvised;
 
         public BreakpointHitHandler(IVsDebugger debugger, BreakpointExtraDataStore extraDataStore, ExceptionBreakManager breakManager, IDiagnosticLogger logger) {
             _debugger = debugger;
             _extraDataStore = extraDataStore;
             _breakManager = breakManager;
             _logger = logger;
 
-            debugger.AdviseDebugEventCallback(this);
+            // Optimization: I expect that most ppl would not use breakpoints feature,
+            // so here we ignore events unless there are actual breakpoints
+            ClassifyAllExtraData();
+            UpdateEventSubscription();
+
+            _extraDataStore.DataLoaded += (sender, e) => {
+                ClassifyAllExtraData();
+                UpdateEventSubscription();
+            };
+            _extraDataStore.DataChanged += (sender, e) => {
+                if (e.Data.ExceptionBreakChange != ExceptionBreakChange.NoChange) {
+                    _actionableExtraData.Add(e.Data);
+                }
+                else {
+                    _actionableExtraData.Remove(e.Data);
+                }
+                UpdateEventSubscription();
+            };
+        }
+
+        private void ClassifyAllExtraData() {
+            _actionableExtraData.Clear();
+            foreach (var data in _extraDataStore.GetAllCurrentData()) {
+                if (data.ExceptionBreakChange != ExceptionBreakChange.NoChange)
+                    _actionableExtraData.Add(data);
+            }
+        }
+
+        private void UpdateEventSubscription() {
+            if (_actionableExtraData.Count > 0) {
+                if (!_eventsAdvised) {
+                    _logger.WriteLine("Found actionable breakpoints, activating breakpoint events.");
+                    _debugger.AdviseDebugEventCallback(this);
+                    _eventsAdvised = true;
+                }
+            }
+            else {
+                if (_eventsAdvised) {
+                    _logger.WriteLine("No actionable breakpoints, deactivating breakpoint events.");
+                    _debugger.UnadviseDebugEventCallback(this);
+                    _eventsAdvised = false;
+                }
+            }
         }
 
         public int Event(IDebugEngine2 pEngine, IDebugProcess2 pProcess, IDebugProgram2 pProgram, IDebugThread2 pThread, IDebugEvent2 pEvent, ref Guid riidEvent, uint dwAttrib) {
@@ -44,13 +88,12 @@ private void ProcessEvent(IDebugEvent2 pEvent) {
             if (breakpointEvent == null)
                 return;
 
-            _logger.WriteLine("Event: Breakpoint reached.");
             foreach (var breakpoint in breakpointEvent.GetBreakpointsAsArraySafe()) {
                 var extraData = _extraDataStore.GetData(breakpoint);
                 if (extraData == null)
                     continue;
 
-                var change = extraData.ExceptionBreakChange.Value;
+                var change = extraData.ExceptionBreakChange;
                 if (change == ExceptionBreakChange.NoChange)
                     continue;
 
@@ -62,7 +105,8 @@ private void ProcessEvent(IDebugEvent2 pEvent) {
         }
 
         public void Dispose() {
-            _debugger.UnadviseDebugEventCallback(this);
+            if (_eventsAdvised)
+                _debugger.UnadviseDebugEventCallback(this);
         }
     }
 }
diff --git a/ExceptionBreaker/Breakpoints/Glyphs/BreakpointGlyphFactory.cs b/ExceptionBreaker/Breakpoints/Glyphs/BreakpointGlyphFactory.cs
@@ -39,7 +39,7 @@ public UIElement GenerateGlyph(IWpfTextViewLine line, IGlyphTag tag) {
                 return null;
 
             return new Image {
-                Source = _images[breakpointTag.BreakpointExtraData.ExceptionBreakChange.Value].Value,
+                Source = _images[breakpointTag.BreakpointExtraData.ExceptionBreakChange].Value,
                 Margin = new Thickness {
                     Left = _breakpointGlyphSize - OverlayGlyphSize,
                     Top = _breakpointGlyphSize - OverlayGlyphSize

diff --git a/ExceptionBreaker/Breakpoints/Glyphs/BreakpointTagger.cs b/ExceptionBreaker/Breakpoints/Glyphs/BreakpointTagger.cs
@@ -28,7 +28,7 @@ public IEnumerable<ITagSpan<BreakpointTag>> GetTags(NormalizedSnapshotSpanCollec
                     continue;
 
                 var extraData = _extraDataStore.GetData(breakpoint);
-                if (extraData.ExceptionBreakChange.Value == ExceptionBreakChange.NoChange)
+                if (extraData.ExceptionBreakChange == ExceptionBreakChange.NoChange)
                     continue;
 
                 yield return new TagSpan<BreakpointTag>(new SnapshotSpan(span.Start, span.Length), new BreakpointTag(extraData));

diff --git a/ExceptionBreaker/ExceptionBreakerPackage.cs b/ExceptionBreaker/ExceptionBreakerPackage.cs
@@ -79,6 +79,7 @@ protected override void Initialize() {
 
         private void SetupCoreManager() {
             var debugger = Mef.GetExportedValue<IDebuggerInternalAdapter>();
+            // TODO: MEF
             var sessionManager = new DebugSessionManager(debugger, Logger);
             var optionsPage = new Lazy<OptionsPageData>(() => (OptionsPageData)GetDialogPage(typeof (OptionsPageData)));
             ExceptionBreakManager = new ExceptionBreakManager(
@@ -89,6 +90,7 @@ private void SetupCoreManager() {
         }
 
         private void SetupToolbar() {
+            // TODO: MEF
             var menuCommandService = GetService(typeof(IMenuCommandService)) as OleMenuCommandService;
             var monitorSelection = (IVsMonitorSelection)GetService(typeof(IVsMonitorSelection));
             _toolbarController = new ToggleBreakOnAllController(
@@ -108,6 +110,7 @@ private void SetupBreakpoints() {
             var extraDataStore = Mef.GetExportedValue<BreakpointExtraDataStore>();
             _solutionDataPersisters.Add(extraDataStore);
 
+            // TODO: MEF
             _breakpointHitHandler = new BreakpointHitHandler(debugger, extraDataStore, ExceptionBreakManager, Logger);
             _breakpointController = new BreakpointExceptionsController(
                 new CommandInitializer(CommandIDs.BreakpointToggleExceptions, menuCommandService),