Skip to content
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.

Sign up for GitHub

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

Dynamo UI should not block during concurrent save/run operations- attempt 1 #12564

Merged
merged 7 commits into from
Jan 21, 2022
Merged

Dynamo UI should not block during concurrent save/run operations- attempt 1 #12564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f8054e2
attempt 1, this works but seems odd.
mjkkirschner Jan 20, 2022
61cec5b
switch executingTask for RunEnabled
mjkkirschner Jan 21, 2022
25d378a
put external files back in wsm
mjkkirschner Jan 21, 2022
5c13fda
int to private
mjkkirschner Jan 21, 2022
081354f
remove usings
mjkkirschner Jan 21, 2022
dba9b84
revert change
mjkkirschner Jan 21, 2022
7bc8f84
revert change
mjkkirschner Jan 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,8 @@ private void OnPreviewGraphCompleted(AsyncTask asyncTask)
OnSetNodeDeltaState(deltaComputeStateArgs);
}
}



#endregion

Expand Down
108 changes: 57 additions & 51 deletions src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,17 @@ public override bool Equals(object obj)
this.WidthAdjustment == other.WidthAdjustment &&
this.HeightAdjustment == other.HeightAdjustment;

//TODO try to get rid of these if possible
//needs investigation if we are okay letting them get
//calculated at runtime. currently checking them will fail as we do
//not deserialize them.

//tolerantDoubleCompare(this.Left, other.Left) &&
//tolerantDoubleCompare(this.Top, other.Top) &&
//tolerantDoubleCompare(this.InitialTop, other.InitialTop);
//this.Width == other.Width &&
//this.Height == other.Height &&
//this.TextBlockHeight == other.TextBlockHeight;
//TODO try to get rid of these if possible
//needs investigation if we are okay letting them get
//calculated at runtime. currently checking them will fail as we do
//not deserialize them.

//tolerantDoubleCompare(this.Left, other.Left) &&
//tolerantDoubleCompare(this.Top, other.Top) &&
//tolerantDoubleCompare(this.InitialTop, other.InitialTop);
//this.Width == other.Width &&
//this.Height == other.Height &&
//this.TextBlockHeight == other.TextBlockHeight;
}
}

Expand Down Expand Up @@ -487,7 +487,7 @@ private void RegisterConnector(ConnectorModel connector)
public event Action<ConnectorModel> ConnectorDeleted;
protected virtual void OnConnectorDeleted(ConnectorModel obj)
{

var handler = ConnectorDeleted;
if (handler != null) handler(obj);
//Check if the workspace is loaded, i.e all the nodes are
Expand Down Expand Up @@ -574,7 +574,7 @@ private void OnSyncWithDefinitionEnd(NodeModel nodeModel)
/// <summary>
/// A set of input parameter states, this can be used to set the graph to a serialized state.
/// </summary>
public IEnumerable<PresetModel> Presets { get { return presets;} }
public IEnumerable<PresetModel> Presets { get { return presets; } }

/// <summary>
/// The date of the last save.
Expand All @@ -595,7 +595,8 @@ public DateTime LastSaved
/// <returns> a list of workspace IDs in GUID form</returns>
public HashSet<Guid> Dependencies
{
get {
get
{
dependencies.Clear();
//if the workspace is a main workspace then find all functions and their dependencies
if (this is HomeWorkspaceModel)
Expand Down Expand Up @@ -673,7 +674,7 @@ internal List<INodeLibraryDependencyInfo> NodeLibraryDependencies
}
// If incorrect version of package is installed and not marked for uninstall,
// set the state. Otherwise, keep the RequiresRestart state away from overwritten.
else if(packageDependencies[saved].State != PackageDependencyState.RequiresRestart)
else if (packageDependencies[saved].State != PackageDependencyState.RequiresRestart)
{
packageDependencies[saved].State = PackageDependencyState.IncorrectVersion;
}
Expand Down Expand Up @@ -723,12 +724,12 @@ internal List<INodeLibraryDependencyInfo> NodeLocalDefinitions
{
get
{
var nodeLocalDefinitions = new Dictionary<object, DependencyInfo>();
var nodeLocalDefinitions = new Dictionary<object, DependencyInfo>();

foreach (var node in Nodes)
{
var collected = GetNodePackage(node);

if (!nodePackageDictionary.ContainsKey(node.GUID) && collected == null)
{
string localDefinitionName;
Expand Down Expand Up @@ -817,13 +818,18 @@ internal List<INodeLibraryDependencyInfo> ExternalFiles
}
}

/// <summary>
/// Computes the external file references if the Workspace Model is a HomeWorkspaceModel and graph is not running.
/// </summary>
/// <returns></returns>
private List<INodeLibraryDependencyInfo> GetExternalFiles()
{
var externalFiles = new Dictionary<object, DependencyInfo>();

// Computes the external file references if the Workspace Model is a HomeWorkspaceModel.
// The workspace should be executed for the external references to be computed because the node output port values are needed.
if (this is HomeWorkspaceModel homeWorkspaceModel)
// If an execution is in progress we'll have to wait for it to be done before we can gather the
// external file references as this implementation relies on the output values of each node.
//instead just bail to avoid blocking the UI.
if (this is HomeWorkspaceModel homeWorkspaceModel && homeWorkspaceModel.RunSettings.RunEnabled)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If retrieving external files is skipped here due to a graph running, say when saving a graph, then what action will cause this computation to actually run again at a later time?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, how do we ensure that this is scheduled to run after the graph finishes executing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently as merged - the user will need to save the graph again, backup save etc while the graph is not running for this computation to run.

We could likely schedule another save if this condition is encountered by using the scheduler. I have some hesitation about this though - if the user tried to manually save their graph as execution starts, and then we schedule another save for directly after - if execution somehow modifies the graph then I guess this could be considered destructive, the example I'm thinking of is element binding, but there could be others... will think about it.

It's also important to note that this issue is only present in sandbox builds (where UI and Scheduler thread are different).

{
foreach (var node in nodes)
{
Expand Down Expand Up @@ -906,9 +912,9 @@ public string Description
/// </summary>
public bool HasUnsavedChanges
{
get
get
{
if(!string.IsNullOrEmpty(this.FileName)) // if there is a filename
if (!string.IsNullOrEmpty(this.FileName)) // if there is a filename
{
if (!File.Exists(this.FileName)) // but the filename is invalid
{
Expand All @@ -930,7 +936,7 @@ public bool HasUnsavedChanges
/// Returns if current workspace is readonly.
/// </summary>
public bool IsReadOnly
{
{
//if the workspace contains xmlDummyNodes it's effectively a readonly graph.
get { return isReadOnly || this.containsXmlDummyNodes() || this.containsInvalidInputSymbols(); }
set
Expand Down Expand Up @@ -970,7 +976,7 @@ private void AddNode(NodeModel node)
{
nodes.Add(node);
}

OnNodeAdded(node);
}

Expand Down Expand Up @@ -1100,7 +1106,7 @@ public double Y
RaisePropertyChanged("Y");
}
}

/// <summary>
/// Get or set the zoom value of the workspace.
/// </summary>
Expand Down Expand Up @@ -1378,7 +1384,7 @@ public virtual void Clear()

ClearUndoRecorder();
ResetWorkspace();

X = 0.0;
Y = 0.0;
Zoom = 1.0;
Expand Down Expand Up @@ -1464,7 +1470,7 @@ protected virtual void RegisterNode(NodeModel node)
{
node.Modified += NodeModified;
node.ConnectorAdded += OnConnectorAdded;
node.UpdateASTCollection +=OnToggleNodeFreeze;
node.UpdateASTCollection += OnToggleNodeFreeze;

var functionNode = node as Function;
if (functionNode != null)
Expand Down Expand Up @@ -1617,8 +1623,8 @@ internal void AddAnnotation(AnnotationModel annotationModel)

internal AnnotationModel AddAnnotation(string text, Guid id)
{
var selectedNodes = this.Nodes == null ? null:this.Nodes.Where(s => s.IsSelected);
var selectedNotes = this.Notes == null ? null: this.Notes.Where(s => s.IsSelected);
var selectedNodes = this.Nodes == null ? null : this.Nodes.Where(s => s.IsSelected);
var selectedNotes = this.Notes == null ? null : this.Notes.Where(s => s.IsSelected);

if (CheckIfModelExistsInSomeGroup(selectedNodes, selectedNotes))
{
Expand All @@ -1642,10 +1648,10 @@ internal AnnotationModel AddAnnotation(string titleText, string text, Guid id)
}

private AnnotationModel CreateAndSubcribeAnnotationModel(
IEnumerable<NodeModel> nodes,
IEnumerable<NoteModel> notes,
Guid id,
string titel,
IEnumerable<NodeModel> nodes,
IEnumerable<NoteModel> notes,
Guid id,
string titel,
string description = "")
{
var annotationModel = new AnnotationModel(nodes, notes)
Expand Down Expand Up @@ -1715,7 +1721,7 @@ private bool CheckIfModelExistsInSomeGroup(IEnumerable<NodeModel> selectNodes, I
foreach (var group in this.Annotations)
{
var groupModels = group.Nodes;

//Selected models minus the ones in the current annotation
var modelsExceptGroup = selectedModels.Except(groupModels).ToList();

Expand Down Expand Up @@ -1758,7 +1764,7 @@ internal IEnumerable<NodeModel> GetSourceNodes()
return
Nodes.Where(
node =>
!node.InPorts.Any()||node.InPorts.All(port => !port.Connectors.Any()));
!node.InPorts.Any() || node.InPorts.All(port => !port.Connectors.Any()));
}

/// <summary>
Expand Down Expand Up @@ -1803,7 +1809,7 @@ internal void IncrementPasteOffset()
/// <returns></returns>
internal bool containsXmlDummyNodes()
{
return this.Nodes.OfType<DummyNode>().Where(node => node.OriginalNodeContent is XmlElement).Count()> 0;
return this.Nodes.OfType<DummyNode>().Where(node => node.OriginalNodeContent is XmlElement).Count() > 0;
}

/// <summary>
Expand Down Expand Up @@ -2237,11 +2243,11 @@ public static WorkspaceModel FromJson(string json, LibraryServices libraryServic
public void UpdateWithExtraWorkspaceViewInfo(ExtraWorkspaceViewInfo workspaceViewInfo)
{
if (workspaceViewInfo == null)
return;
return;

X = workspaceViewInfo.X;
Y = workspaceViewInfo.Y;
Zoom = workspaceViewInfo.Zoom;
Zoom = workspaceViewInfo.Zoom;

OnCurrentOffsetChanged(
this,
Expand Down Expand Up @@ -2278,7 +2284,7 @@ public void UpdateWithExtraWorkspaceViewInfo(ExtraWorkspaceViewInfo workspaceVie
private void LoadNodes(IEnumerable<ExtraNodeViewInfo> nodeViews)
{
if (nodeViews == null)
return;
return;

foreach (ExtraNodeViewInfo nodeViewInfo in nodeViews)
{
Expand All @@ -2301,9 +2307,9 @@ private void LoadNodes(IEnumerable<ExtraNodeViewInfo> nodeViews)
nodeModel.UpdateValue(new UpdateValueParams("IsVisible", nodeViewInfo.ShowGeometry.ToString()));
}
else
{
this.Log(string.Format("This graph has a nodeview with id:{0} and name:{1}, but does not contain a matching nodeModel",
guidValue.ToString(),nodeViewInfo.Name)
{
this.Log(string.Format("This graph has a nodeview with id:{0} and name:{1}, but does not contain a matching nodeModel",
guidValue.ToString(), nodeViewInfo.Name)
, WarningLevel.Moderate);
}
}
Expand All @@ -2312,7 +2318,7 @@ private void LoadNodes(IEnumerable<ExtraNodeViewInfo> nodeViews)
private void LoadLegacyNotes(IEnumerable<ExtraNoteViewInfo> noteViews)
{
if (noteViews == null)
return;
return;

foreach (ExtraNoteViewInfo noteViewInfo in noteViews)
{
Expand All @@ -2333,7 +2339,7 @@ private void LoadLegacyNotes(IEnumerable<ExtraNoteViewInfo> noteViews)
private void LoadNotesFromAnnotations(IEnumerable<ExtraAnnotationViewInfo> annotationViews)
{
if (annotationViews == null)
return;
return;

foreach (ExtraAnnotationViewInfo annotationViewInfo in annotationViews)
{
Expand All @@ -2351,9 +2357,9 @@ private void LoadNotesFromAnnotations(IEnumerable<ExtraAnnotationViewInfo> annot
FirstOrDefault(x => x.GUID.ToString("N") == annotationViewInfo.PinnedNode);

var noteModel = new NoteModel(
annotationViewInfo.Left,
annotationViewInfo.Top,
text,
annotationViewInfo.Left,
annotationViewInfo.Top,
text,
annotationGuidValue,
pinnedNode);

Expand All @@ -2368,14 +2374,14 @@ private void LoadNotesFromAnnotations(IEnumerable<ExtraAnnotationViewInfo> annot

private void LoadConnectorPins(IEnumerable<ExtraConnectorPinInfo> pinInfo)
{
if (pinInfo == null) {return;}
if (pinInfo == null) { return; }

foreach (ExtraConnectorPinInfo pinViewInfo in pinInfo)
{
var connectorGuid = IdToGuidConverter(pinViewInfo.ConnectorGuid);

var matchingConnector = Connectors.FirstOrDefault(x => x.GUID == connectorGuid);
if (matchingConnector is null) {return;}
if (matchingConnector is null) { return; }

matchingConnector.AddPin(pinViewInfo.Left, pinViewInfo.Top);
}
Expand All @@ -2392,7 +2398,7 @@ private void LoadAnnotations(IEnumerable<ExtraAnnotationViewInfo> annotationView
// Before creating this group we need to create
// any group belonging to this group.
if (annotationViewInfo.HasNestedGroups &&
!annotationQueue.All(x=>x.HasNestedGroups))
!annotationQueue.All(x => x.HasNestedGroups))
{
annotationQueue.Enqueue(annotationViewInfo);
continue;
Expand Down Expand Up @@ -2494,7 +2500,7 @@ private Guid IdToGuidConverter(string id)

return deterministicGuid;
}

/// <summary>
/// Returns a DelayedGraphExecution object.
/// </summary>
Expand Down
9 changes: 1 addition & 8 deletions src/DynamoCore/Logging/DynamoAnalyticsClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace Dynamo.Logging
{
class DynamoAnalyticsSession : IAnalyticsSession
{
private Heartbeat heartbeat;
private UsageLog logger;

public DynamoAnalyticsSession()
Expand All @@ -25,8 +24,6 @@ public void Start(DynamoModel model)
{
StabilityCookie.Startup();

heartbeat = Heartbeat.GetInstance(model);

logger = new UsageLog("Dynamo", UserId, SessionId);
}

Expand All @@ -38,10 +35,6 @@ public void Dispose()
else
StabilityCookie.WriteCleanShutdown();

if (null != heartbeat)
Heartbeat.DestroyInstance();
heartbeat = null;

if (null != logger)
logger.Dispose();
logger = null;
Expand Down Expand Up @@ -178,7 +171,7 @@ public DynamoAnalyticsClient(DynamoModel dynamoModel)

if (Session == null) Session = new DynamoAnalyticsSession();

//Setup Analytics service, StabilityCookie, Heartbeat and UsageLog.
//Setup Analytics service, StabilityCookie, and UsageLog.
Session.Start(dynamoModel);

//Dynamo app version.
Expand Down
Loading