From 432388de60e09c8af8abc2daa5bc9ea264ccbe76 Mon Sep 17 00:00:00 2001 From: Benjamin Popp Date: Thu, 13 Jun 2019 22:10:44 -0500 Subject: [PATCH] Caching Bugfix * Only allow one scope to be created at a time, to avoid race conditions with modifying the activeScopes. * Allow scopes to be created for the same model when a scope already exists. Use reference counting in this case. * Change at what level several of the scopes are created at. --- src/HexManiac.Core/Models/IDataModel.cs | 4 +- src/HexManiac.Core/Models/PokemonModel.cs | 20 ++--- .../Models/Runs/ModelCacheScope.cs | 20 +++-- .../ViewModels/Tools/IToolTrayViewModel.cs | 11 ++- .../ViewModels/Tools/PCSTool.cs | 10 ++- src/HexManiac.Core/ViewModels/ViewPort.cs | 80 ++++++++++--------- .../Visitors/CompleteEditOperation.cs | 4 +- 7 files changed, 86 insertions(+), 63 deletions(-) diff --git a/src/HexManiac.Core/Models/IDataModel.cs b/src/HexManiac.Core/Models/IDataModel.cs index 2933ba5a..60fabee8 100644 --- a/src/HexManiac.Core/Models/IDataModel.cs +++ b/src/HexManiac.Core/Models/IDataModel.cs @@ -7,7 +7,7 @@ using System.Collections.Generic; using System.Linq; namespace HavenSoft.HexManiac.Core.Models { - public interface IDataModel : IReadOnlyList { + public interface IDataModel : IReadOnlyList, IEquatable { byte[] RawData { get; } new byte this[int index] { get; set; } IReadOnlyList Arrays { get; } @@ -138,6 +138,8 @@ namespace HavenSoft.HexManiac.Core.Models { public virtual StoredMetadata ExportMetadata() => null; IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + public bool Equals(IDataModel other) => other == this; } public static class IDataModelExtensions { diff --git a/src/HexManiac.Core/Models/PokemonModel.cs b/src/HexManiac.Core/Models/PokemonModel.cs index 746cf700..ae5110da 100644 --- a/src/HexManiac.Core/Models/PokemonModel.cs +++ b/src/HexManiac.Core/Models/PokemonModel.cs @@ -440,16 +440,14 @@ namespace HavenSoft.HexManiac.Core.Models { private void ModifyAnchorsFromPointerArray(ModelDelta changeToken, ArrayRun arrayRun, Action changeAnchors) { int segmentOffset = arrayRun.Start; // i loops over the different segments in the array - using (ModelCacheScope.CreateScope(this)) { - for (int i = 0; i < arrayRun.ElementContent.Count; i++) { - if (arrayRun.ElementContent[i].Type != ElementContentType.Pointer) { segmentOffset += arrayRun.ElementContent[i].Length; continue; } - // for a pointer segment, j loops over all the elements in the array - for (int j = 0; j < arrayRun.ElementCount; j++) { - var start = segmentOffset + arrayRun.ElementLength * j; - changeAnchors(arrayRun.ElementContent[i], changeToken, start); - } - segmentOffset += arrayRun.ElementContent[i].Length; + for (int i = 0; i < arrayRun.ElementContent.Count; i++) { + if (arrayRun.ElementContent[i].Type != ElementContentType.Pointer) { segmentOffset += arrayRun.ElementContent[i].Length; continue; } + // for a pointer segment, j loops over all the elements in the array + for (int j = 0; j < arrayRun.ElementCount; j++) { + var start = segmentOffset + arrayRun.ElementLength * j; + changeAnchors(arrayRun.ElementContent[i], changeToken, start); } + segmentOffset += arrayRun.ElementContent[i].Length; } } @@ -566,7 +564,9 @@ namespace HavenSoft.HexManiac.Core.Models { if (run is ArrayRun array && array.SupportsPointersToElements) run = array.AddSourcesPointingWithinArray(changeToken); var newRun = run.MergeAnchor(sources); - ObserveRunWritten(changeToken, newRun); + using (ModelCacheScope.CreateScope(this)) { + ObserveRunWritten(changeToken, newRun); + } } public override void MassUpdateFromDelta(IReadOnlyDictionary runsToRemove, IReadOnlyDictionary runsToAdd, IReadOnlyDictionary namesToRemove, IReadOnlyDictionary namesToAdd, IReadOnlyDictionary unmappedPointersToRemove, IReadOnlyDictionary unmappedPointersToAdd) { diff --git a/src/HexManiac.Core/Models/Runs/ModelCacheScope.cs b/src/HexManiac.Core/Models/Runs/ModelCacheScope.cs index bbceb07e..fc0e5e85 100644 --- a/src/HexManiac.Core/Models/Runs/ModelCacheScope.cs +++ b/src/HexManiac.Core/Models/Runs/ModelCacheScope.cs @@ -1,18 +1,28 @@ using System; using System.Collections.Generic; -using System.Diagnostics; namespace HavenSoft.HexManiac.Core.Models.Runs { public class ModelCacheScope : IDisposable { private static readonly Dictionary activeScopes = new Dictionary(); + private readonly IDataModel model; + private int references; private ModelCacheScope(IDataModel model) => this.model = model; public static IDisposable CreateScope(IDataModel model) { - Debug.Assert(!activeScopes.ContainsKey(model)); - activeScopes[model] = new ModelCacheScope(model); - return activeScopes[model]; + lock (activeScopes) { + if (!activeScopes.ContainsKey(model)) { + activeScopes[model] = new ModelCacheScope(model); + } + activeScopes[model].references++; + return activeScopes[model]; + } + } + public void Dispose() { + lock (activeScopes) { + activeScopes[model].references--; + if (activeScopes[model].references == 0) activeScopes.Remove(model); + } } - public void Dispose() => activeScopes.Remove(model); public static ModelCacheScope GetCache(IDataModel model) => activeScopes.TryGetValue(model, out var scope) ? scope : null; diff --git a/src/HexManiac.Core/ViewModels/Tools/IToolTrayViewModel.cs b/src/HexManiac.Core/ViewModels/Tools/IToolTrayViewModel.cs index d7d6e5f7..bbe37e21 100644 --- a/src/HexManiac.Core/ViewModels/Tools/IToolTrayViewModel.cs +++ b/src/HexManiac.Core/ViewModels/Tools/IToolTrayViewModel.cs @@ -1,4 +1,5 @@ using HavenSoft.HexManiac.Core.Models; +using HavenSoft.HexManiac.Core.Models.Runs; using System; using System.Collections; using System.Collections.Generic; @@ -33,6 +34,7 @@ namespace HavenSoft.HexManiac.Core.ViewModels.Tools { private readonly StubCommand hideCommand; private readonly StubCommand stringToolCommand, tableToolCommand, tool3Command; private readonly HashSet deferredWork = new HashSet(); + private readonly IDataModel model; private int selectedIndex; public int SelectedIndex { @@ -65,9 +67,11 @@ namespace HavenSoft.HexManiac.Core.ViewModels.Tools { Debug.Assert(currentDeferralToken == null); currentDeferralToken = new StubDisposable { Dispose = () => { - foreach (var action in deferredWork) action(); - deferredWork.Clear(); - currentDeferralToken = null; + using (ModelCacheScope.CreateScope(model)) { + foreach (var action in deferredWork) action(); + deferredWork.Clear(); + currentDeferralToken = null; + } } }; return currentDeferralToken; @@ -79,6 +83,7 @@ namespace HavenSoft.HexManiac.Core.ViewModels.Tools { public event EventHandler RequestMenuClose; public ToolTray(IDataModel model, Selection selection, ChangeHistory history) { + this.model = model; tools = new IToolViewModel[] { new PCSTool(model, selection, history, this), new TableTool(model, selection, history, this), diff --git a/src/HexManiac.Core/ViewModels/Tools/PCSTool.cs b/src/HexManiac.Core/ViewModels/Tools/PCSTool.cs index 34f5c370..f4b0bb53 100644 --- a/src/HexManiac.Core/ViewModels/Tools/PCSTool.cs +++ b/src/HexManiac.Core/ViewModels/Tools/PCSTool.cs @@ -169,10 +169,12 @@ namespace HavenSoft.HexManiac.Core.ViewModels.Tools { } return; } else if (run is IStreamRun stream) { - var newContent = stream.SerializeRun(); - ignoreSelectionUpdates = true; - using (new StubDisposable { Dispose = () => ignoreSelectionUpdates = false }) { - TryUpdate(ref content, newContent, nameof(Content)); + using (ModelCacheScope.CreateScope(model)) { + var newContent = stream.SerializeRun(); + ignoreSelectionUpdates = true; + using (new StubDisposable { Dispose = () => ignoreSelectionUpdates = false }) { + TryUpdate(ref content, newContent, nameof(Content)); + } } return; } diff --git a/src/HexManiac.Core/ViewModels/ViewPort.cs b/src/HexManiac.Core/ViewModels/ViewPort.cs index badc57cb..342f169e 100644 --- a/src/HexManiac.Core/ViewModels/ViewPort.cs +++ b/src/HexManiac.Core/ViewModels/ViewPort.cs @@ -1034,29 +1034,31 @@ namespace HavenSoft.HexManiac.Core.ViewModels { } private IReadOnlyList GetAutocompleteOptions(IDataFormat originalFormat, string newText, int selectedIndex = -1) { - if (originalFormat is Anchor anchor) originalFormat = anchor.OriginalFormat; - if (newText.StartsWith(PointerStart.ToString())) { - return Model.GetNewPointerAutocompleteOptions(newText, selectedIndex); - } else if (newText.StartsWith(GotoMarker.ToString())) { - return Model.GetNewPointerAutocompleteOptions(newText, selectedIndex); - } else if (originalFormat is IntegerEnum intEnum) { - var array = (ArrayRun)Model.GetNextRun(intEnum.Source); - var segment = (ArrayRunEnumSegment)array.ElementContent[array.ConvertByteOffsetToArrayOffset(intEnum.Source).SegmentIndex]; - var options = segment.GetOptions(Model).Select(option => option + " "); // autocomplete needs to complete after selection, so add a space - return AutoCompleteSelectionItem.Generate(options.Where(option => option.MatchesPartial(newText)), selectedIndex); - } else if (originalFormat is EggSection || originalFormat is EggItem) { - var eggRun = (EggMoveRun)Model.GetNextRun(((IDataFormatInstance)originalFormat).Source); - var allOptions = eggRun.GetAutoCompleteOptions(); - return AutoCompleteSelectionItem.Generate(allOptions.Where(option => option.MatchesPartial(newText)), selectedIndex); - } else if (originalFormat is PlmItem) { - if (!newText.Contains(" ")) return AutoCompleteSelectionItem.Generate(Enumerable.Empty(), -1); - var moveName = newText.Substring(newText.IndexOf(' ')).Trim(); - if (moveName.Length == 0) return AutoCompleteSelectionItem.Generate(Enumerable.Empty(), -1); - var plmRun = (PLMRun)Model.GetNextRun(((IDataFormatInstance)originalFormat).Source); - var allOptions = plmRun.GetAutoCompleteOptions(newText.Split(' ')[0]); - return AutoCompleteSelectionItem.Generate(allOptions.Where(option => option.MatchesPartial(moveName)), selectedIndex); - } else { - throw new NotImplementedException(); + using (ModelCacheScope.CreateScope(Model)) { + if (originalFormat is Anchor anchor) originalFormat = anchor.OriginalFormat; + if (newText.StartsWith(PointerStart.ToString())) { + return Model.GetNewPointerAutocompleteOptions(newText, selectedIndex); + } else if (newText.StartsWith(GotoMarker.ToString())) { + return Model.GetNewPointerAutocompleteOptions(newText, selectedIndex); + } else if (originalFormat is IntegerEnum intEnum) { + var array = (ArrayRun)Model.GetNextRun(intEnum.Source); + var segment = (ArrayRunEnumSegment)array.ElementContent[array.ConvertByteOffsetToArrayOffset(intEnum.Source).SegmentIndex]; + var options = segment.GetOptions(Model).Select(option => option + " "); // autocomplete needs to complete after selection, so add a space + return AutoCompleteSelectionItem.Generate(options.Where(option => option.MatchesPartial(newText)), selectedIndex); + } else if (originalFormat is EggSection || originalFormat is EggItem) { + var eggRun = (EggMoveRun)Model.GetNextRun(((IDataFormatInstance)originalFormat).Source); + var allOptions = eggRun.GetAutoCompleteOptions(); + return AutoCompleteSelectionItem.Generate(allOptions.Where(option => option.MatchesPartial(newText)), selectedIndex); + } else if (originalFormat is PlmItem) { + if (!newText.Contains(" ")) return AutoCompleteSelectionItem.Generate(Enumerable.Empty(), -1); + var moveName = newText.Substring(newText.IndexOf(' ')).Trim(); + if (moveName.Length == 0) return AutoCompleteSelectionItem.Generate(Enumerable.Empty(), -1); + var plmRun = (PLMRun)Model.GetNextRun(((IDataFormatInstance)originalFormat).Source); + var allOptions = plmRun.GetAutoCompleteOptions(newText.Split(' ')[0]); + return AutoCompleteSelectionItem.Generate(allOptions.Where(option => option.MatchesPartial(moveName)), selectedIndex); + } else { + throw new NotImplementedException(); + } } } @@ -1195,7 +1197,9 @@ namespace HavenSoft.HexManiac.Core.ViewModels { // normal case: whether or not to accept the edit depends on the existing cell format var dataIndex = scroll.ViewPointToDataIndex(point); var completeEditOperation = new CompleteEditOperation(Model, dataIndex, underEdit.CurrentText, history.CurrentChange); - underEdit.OriginalFormat.Visit(completeEditOperation, element.Value); + using (ModelCacheScope.CreateScope(Model)) { + underEdit.OriginalFormat.Visit(completeEditOperation, element.Value); + } if (completeEditOperation.Result) { if (completeEditOperation.NewCell != null) { currentView[point.X, point.Y] = completeEditOperation.NewCell; @@ -1296,21 +1300,23 @@ namespace HavenSoft.HexManiac.Core.ViewModels { ErrorInfo errorInfo; // if it's an unnamed text/stream anchor, we have special logic for that - if (underEdit.CurrentText == AnchorStart + PCSRun.SharedFormatString) { - int count = Model.ConsiderResultsAsTextRuns(history.CurrentChange, new[] { index }); - if (count == 0) { - errorInfo = new ErrorInfo("An anchor with nothing pointing to it must have a name."); + using (ModelCacheScope.CreateScope(Model)) { + if (underEdit.CurrentText == AnchorStart + PCSRun.SharedFormatString) { + int count = Model.ConsiderResultsAsTextRuns(history.CurrentChange, new[] { index }); + if (count == 0) { + errorInfo = new ErrorInfo("An anchor with nothing pointing to it must have a name."); + } else { + errorInfo = ErrorInfo.NoError; + } + } else if (underEdit.CurrentText == AnchorStart + PLMRun.SharedFormatString) { + if (!PokemonModel.ConsiderAsPlmStream(Model, index, history.CurrentChange)) { + errorInfo = new ErrorInfo("An anchor with nothing pointing to it must have a name."); + } else { + errorInfo = ErrorInfo.NoError; + } } else { - errorInfo = ErrorInfo.NoError; + errorInfo = PokemonModel.ApplyAnchor(Model, history.CurrentChange, index, underEdit.CurrentText); } - } else if (underEdit.CurrentText == AnchorStart + PLMRun.SharedFormatString) { - if (!PokemonModel.ConsiderAsPlmStream(Model, index, history.CurrentChange)) { - errorInfo = new ErrorInfo("An anchor with nothing pointing to it must have a name."); - } else { - errorInfo = ErrorInfo.NoError; - } - } else { - errorInfo = PokemonModel.ApplyAnchor(Model, history.CurrentChange, index, underEdit.CurrentText); } ClearEdits(point); diff --git a/src/HexManiac.Core/ViewModels/Visitors/CompleteEditOperation.cs b/src/HexManiac.Core/ViewModels/Visitors/CompleteEditOperation.cs index e4e2dda6..a0335eee 100644 --- a/src/HexManiac.Core/ViewModels/Visitors/CompleteEditOperation.cs +++ b/src/HexManiac.Core/ViewModels/Visitors/CompleteEditOperation.cs @@ -258,9 +258,7 @@ namespace HavenSoft.HexManiac.Core.ViewModels.Visitors { if (fullValue == Pointer.NULL || (0 <= fullValue && fullValue < Model.Count)) { if (inArray) { - using (ModelCacheScope.CreateScope(Model)) { - UpdateArrayPointer((ArrayRun)currentRun, fullValue); - } + UpdateArrayPointer((ArrayRun)currentRun, fullValue); } else { Model.WritePointer(CurrentChange, memoryLocation, fullValue); Model.ObserveRunWritten(CurrentChange, new PointerRun(memoryLocation, sources));