Skip to content

Commit

Permalink
[DYN-4517] TuneUp will fail to register if not closed between graph r…
Browse files Browse the repository at this point in the history
…uns (DynamoDS#43)

* TuneUp upgraded to NET8

builds without errors

* .NET8 except binaries

* updated packages

* loads in 3.1+

now loads in all versions from the Extensions menu

* readme

* subscribes only when MenuItem.IsChecked

* Subscribe/unsubscrive called from IsTuneUpActive

* ready for PR

- TuneUp active only when MenuItem.IsChecked
- switches to manual model when activated
- ViewModel.IsTuneUpChecked tracks MenuItem.IsChecked

* conflicts resolved

aligned to .csproj & .props in master
  • Loading branch information
ivaylo-matov authored Jul 2, 2024
1 parent 8a79d36 commit 2fafe30
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 43 deletions.
7 changes: 6 additions & 1 deletion TuneUp/TuneUpViewExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,19 @@ public override void Loaded(ViewLoadedParams p)
if (TuneUpMenuItem.IsChecked)
{
p.AddToExtensionsSideBar(this, TuneUpView);
ViewModel.EnableProfiling();
ViewModel.SwitchToManualMode();
ViewModel.EnableProfiling();
}
else
{
p.CloseExtensioninInSideBar(this);
}
};

// Bind the IsChecked property to the IsTuneUpActive property
TuneUpMenuItem.Checked += (sender, args) => ViewModel.IsTuneUpChecked = true;
TuneUpMenuItem.Unchecked += (sender, args) => ViewModel.IsTuneUpChecked = false;

// Add this view extension's menu item to the Extensions tab or View tab accordingly.
var dynamoMenuItems = p.dynamoMenu.Items.OfType<MenuItem>();
// TODO: Investigate why TuneUpMenuItem is not added to the specified MenuItem
Expand Down
125 changes: 83 additions & 42 deletions TuneUp/TuneUpWindowViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public enum ProfiledNodeState
public class TuneUpWindowViewModel : NotificationObject, IDisposable
{
#region Internal Properties

private ViewLoadedParams viewLoadedParams;
private IProfilingExecutionTimeData executionTimeData;
private int executedNodesNum;
Expand All @@ -48,6 +49,7 @@ public class TuneUpWindowViewModel : NotificationObject, IDisposable
private HomeWorkspaceModel currentWorkspace;
private Dictionary<Guid, ProfiledNodeViewModel> nodeDictionary = new Dictionary<Guid, ProfiledNodeViewModel>();
private SynchronizationContext uiContext;
private bool isTuneUpChecked = false;

/// <summary>
/// Name of the row to display current execution time
Expand All @@ -71,30 +73,31 @@ public class TuneUpWindowViewModel : NotificationObject, IDisposable

private HomeWorkspaceModel CurrentWorkspace
{
get
{
return currentWorkspace;
}
get => currentWorkspace;
set
{
// Unsubscribe from old workspace
if (currentWorkspace != null)
if (currentWorkspace != null && isTuneUpChecked)
{
UnsubscribeWorkspaceEvents(currentWorkspace);
ManageWorkspaceEvents(currentWorkspace, false);
}

// Subscribe to new workspace
if (value != null)
{
// Set new workspace
currentWorkspace = value;
SubscribeWorkspaceEvents(currentWorkspace);
if (isTuneUpChecked)
{
ManageWorkspaceEvents(currentWorkspace, true);
}
}
}
}
#endregion

#region Public Properties

/// <summary>
/// Is the recomputeAll button enabled in the UI. Users should not be able to force a
/// reset of the engine and re-execution of the graph if one is still ongoing. This causes...trouble.
Expand All @@ -111,6 +114,31 @@ private set
}
}
}

/// <summary>
/// Gets or sets a value indicating whether the TuneUp extension is active.
/// When activated, it subscribes to workspace events to enable profiling.
/// When deactivated, it unsubscribes from those events.
/// </summary>
public bool IsTuneUpChecked
{
get => isTuneUpChecked;
set
{
if (isTuneUpChecked != value)
{
isTuneUpChecked = value;
RaisePropertyChanged(nameof(IsTuneUpChecked));

// Subscribe if activated, unsubscribe if deactivated
if (currentWorkspace != null)
{
ManageWorkspaceEvents(currentWorkspace, isTuneUpChecked);
}
}
}
}

/// <summary>
/// Collection of profiling data for nodes in the current workspace
/// </summary>
Expand Down Expand Up @@ -153,11 +181,14 @@ public TuneUpWindowViewModel(ViewLoadedParams p)
CurrentWorkspace = p.CurrentWorkspaceModel as HomeWorkspaceModel;
}
}

#endregion

#region ProfilingMethods

/// <summary>
/// Resets the profiling data for all nodes in the current workspace. Clears the existing
/// profiling data and re-initializes it based on the nodes present in the current workspace.
/// </summary>
internal void ResetProfiledNodes()
{
if (CurrentWorkspace == null)
Expand Down Expand Up @@ -186,13 +217,13 @@ internal void ResetProfiledNodes()
}

/// <summary>
/// The hanlder for force-recompute the graph
/// The handler for force-recompute the graph
/// </summary>
internal void ResetProfiling()
{
// Put the graph into manual mode as there is no guarantee that nodes will be marked
// dirty in topologically sorted oreder during a reset.
CurrentWorkspace.RunSettings.RunType = Dynamo.Models.RunType.Manual;
// dirty in topologically sorted order during a reset.
SwitchToManualMode();
// TODO: need a way to do this from an extension and not cause a run.
// DynamoModel interface or a more specific reset command.
(viewLoadedParams.DynamoWindow.DataContext as DynamoViewModel).Model.ResetEngine(true);
Expand All @@ -205,6 +236,15 @@ internal void ResetProfiling()
executionTimeData = CurrentWorkspace.EngineController.ExecutionTimeData;
}

/// <summary>
/// Switches the current workspace's run mode to manual. Prevents the workspace from
/// running automatically and allows for manual control of execution.
/// </summary>
internal void SwitchToManualMode()
{
CurrentWorkspace.RunSettings.RunType = Dynamo.Models.RunType.Manual;
}

/// <summary>
/// Enable profiling when it is disabled temporarily.
/// </summary>
Expand Down Expand Up @@ -357,42 +397,43 @@ private void OnCurrentWorkspaceCleared(IWorkspaceModel workspace)

/// <summary>
/// When switching workspaces or closing TuneUp extension,
/// unsubscribe workspace events for profiling
/// subscribe (true)/unsubscribe (false) workspace events for profiling
/// </summary>
/// <param name="workspace">target workspace</param>
private void UnsubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
/// <param name="workspace"></param>
/// <param name="subscribe"></param>
private void ManageWorkspaceEvents(HomeWorkspaceModel workspace, bool subscribe)
{
workspace.NodeAdded -= CurrentWorkspaceModel_NodeAdded;
workspace.NodeRemoved -= CurrentWorkspaceModel_NodeRemoved;
workspace.EvaluationStarted -= CurrentWorkspaceModel_EvaluationStarted;
workspace.EvaluationCompleted -= CurrentWorkspaceModel_EvaluationCompleted;
if (workspace == null) return;

foreach (var node in workspace.Nodes)
// Subscribe from workspace events
if (subscribe)
{
node.NodeExecutionBegin -= OnNodeExecutionBegin;
node.NodeExecutionEnd -= OnNodeExecutionEnd;
}
executedNodesNum = 0;
}

/// <summary>
/// When switching workspaces or closing TuneUp extension,
/// subscribe workspace events for profiling
/// </summary>
/// <param name="workspace">target workspace</param>
private void SubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
{
workspace.NodeAdded += CurrentWorkspaceModel_NodeAdded;
workspace.NodeRemoved += CurrentWorkspaceModel_NodeRemoved;
workspace.EvaluationStarted += CurrentWorkspaceModel_EvaluationStarted;
workspace.EvaluationCompleted += CurrentWorkspaceModel_EvaluationCompleted;
workspace.NodeAdded += CurrentWorkspaceModel_NodeAdded;
workspace.NodeRemoved += CurrentWorkspaceModel_NodeRemoved;
workspace.EvaluationStarted += CurrentWorkspaceModel_EvaluationStarted;
workspace.EvaluationCompleted += CurrentWorkspaceModel_EvaluationCompleted;

foreach (var node in workspace.Nodes)
{
node.NodeExecutionBegin += OnNodeExecutionBegin;
node.NodeExecutionEnd += OnNodeExecutionEnd;
foreach (var node in workspace.Nodes)
{
node.NodeExecutionBegin += OnNodeExecutionBegin;
node.NodeExecutionEnd += OnNodeExecutionEnd;
}
ResetProfiledNodes();
}
ResetProfiledNodes();
// Unsubscribe to workspace events
else
{
workspace.NodeAdded -= CurrentWorkspaceModel_NodeAdded;
workspace.NodeRemoved -= CurrentWorkspaceModel_NodeRemoved;
workspace.EvaluationStarted -= CurrentWorkspaceModel_EvaluationStarted;
workspace.EvaluationCompleted -= CurrentWorkspaceModel_EvaluationCompleted;

foreach (var node in workspace.Nodes)
{
node.NodeExecutionBegin -= OnNodeExecutionBegin;
node.NodeExecutionEnd -= OnNodeExecutionEnd;
}
}
executedNodesNum = 0;
}

Expand All @@ -401,7 +442,7 @@ private void SubscribeWorkspaceEvents(HomeWorkspaceModel workspace)
/// </summary>
public void Dispose()
{
UnsubscribeWorkspaceEvents(CurrentWorkspace);
ManageWorkspaceEvents(CurrentWorkspace, false);
viewLoadedParams.CurrentWorkspaceChanged -= OnCurrentWorkspaceChanged;
viewLoadedParams.CurrentWorkspaceCleared -= OnCurrentWorkspaceCleared;
}
Expand Down

0 comments on commit 2fafe30

Please sign in to comment.