Skip to content

Commit

Permalink
Further imrpove RamSearch performance
Browse files Browse the repository at this point in the history
- switch _watchList from a List to an array
- more Domain.EnterExit usage
  • Loading branch information
Morilli committed Jul 4, 2022
1 parent 9e90290 commit 011f4bf
Showing 1 changed file with 85 additions and 79 deletions.
164 changes: 85 additions & 79 deletions src/BizHawk.Client.Common/tools/RamSearchEngine/RamSearchEngine.cs
Expand Up @@ -14,10 +14,10 @@ public class RamSearchEngine
{
private Compare _compareTo = Compare.Previous;

private List<IMiniWatch> _watchList = new List<IMiniWatch>();
private IMiniWatch[] _watchList;
private readonly SearchEngineSettings _settings;
private readonly UndoHistory<IEnumerable<IMiniWatch>> _history = new UndoHistory<IEnumerable<IMiniWatch>>(true, new List<IMiniWatch>()); //TODO use IList instead of IEnumerable and stop calling `.ToList()` (i.e. cloning) on reads and writes?
private bool _isSorted = true; // Tracks whether or not the list is sorted by address, if it is, binary search can be used for finding watches
private readonly UndoHistory<IEnumerable<IMiniWatch>> _history = new UndoHistory<IEnumerable<IMiniWatch>>(true, new List<IMiniWatch>()); //TODO use IList instead of IEnumerable and stop calling `.ToArray()` (i.e. cloning) on reads and writes?
private bool _isSorted = true; // Tracks whether or not the array is sorted by address, if it is, binary search can be used for finding watches

public RamSearchEngine(SearchEngineSettings settings, IMemoryDomains memoryDomains)
{
Expand Down Expand Up @@ -55,53 +55,56 @@ public void Start()
listSize /= (int)_settings.Size;
}

_watchList = new List<IMiniWatch>((int)listSize);
_watchList = new IMiniWatch[listSize];

switch (_settings.Size)
using (Domain.EnterExit())

This comment has been minimized.

Copy link
@YoshiRulz

YoshiRulz Jul 4, 2022

Member

IIRC you can do using var _ = Domain.EnterExit(); to avoid indentation? Obviously it delays the disposal until the end of the scope but it would have been the same in this case.

This comment has been minimized.

Copy link
@Morilli

Morilli Jul 4, 2022

Author Collaborator

Yeah I could have, but didn't like the var _ syntax too much (looks weird, considering you usually don't want to "discard" the variable you're using).

{
default:
case WatchSize.Byte:
for (int i = 0; i < domain.Size; i++)
{
if (_settings.IsDetailed())
{
_watchList.Add(new MiniByteWatchDetailed(domain, i));
}
else
switch (_settings.Size)
{
default:
case WatchSize.Byte:
for (int i = 0; i < domain.Size; i++)
{
_watchList.Add(new MiniByteWatch(domain, i));
if (_settings.IsDetailed())
{
_watchList[i] = new MiniByteWatchDetailed(domain, i);
}
else
{
_watchList[i] = new MiniByteWatch(domain, i);
}
}
}

break;
case WatchSize.Word:
for (int i = 0; i < domain.Size - 1; i += _settings.CheckMisAligned ? 1 : 2)
{
if (_settings.IsDetailed())
{
_watchList.Add(new MiniWordWatchDetailed(domain, i, _settings.BigEndian));
}
else
break;
case WatchSize.Word:
for (int i = 0; i < domain.Size - 1; i += _settings.CheckMisAligned ? 1 : 2)
{
_watchList.Add(new MiniWordWatch(domain, i, _settings.BigEndian));
if (_settings.IsDetailed())
{
_watchList[i] = new MiniWordWatchDetailed(domain, i, _settings.BigEndian);
}
else
{
_watchList[i] = new MiniWordWatch(domain, i, _settings.BigEndian);
}
}
}

break;
case WatchSize.DWord:
for (int i = 0; i < domain.Size - 3; i += _settings.CheckMisAligned ? 1 : 4)
{
if (_settings.IsDetailed())
break;
case WatchSize.DWord:
for (int i = 0; i < domain.Size - 3; i += _settings.CheckMisAligned ? 1 : 4)
{
_watchList.Add(new MiniDWordWatchDetailed(domain, i, _settings.BigEndian));
}
else
{
_watchList.Add(new MiniDWordWatch(domain, i, _settings.BigEndian));
if (_settings.IsDetailed())
{
_watchList[i] = new MiniDWordWatchDetailed(domain, i, _settings.BigEndian);
}
else
{
_watchList[i] = new MiniDWordWatch(domain, i, _settings.BigEndian);
}
}
}

break;
break;
}
}
}

Expand All @@ -122,29 +125,32 @@ public void Start()

public int DoSearch()
{
int before = _watchList.Count;
int before = _watchList.Length;

_watchList = _compareTo switch
using (Domain.EnterExit())
{
Compare.Previous => ComparePrevious(_watchList).ToList(),
Compare.SpecificValue => CompareSpecificValue(_watchList).ToList(),
Compare.SpecificAddress => CompareSpecificAddress(_watchList).ToList(),
Compare.Changes => CompareChanges(_watchList).ToList(),
Compare.Difference => CompareDifference(_watchList).ToList(),
_ => ComparePrevious(_watchList).ToList()
};

if (_settings.PreviousType == PreviousType.LastSearch)
{
SetPreviousToCurrent();
_watchList = _compareTo switch
{
Compare.Previous => ComparePrevious(_watchList).ToArray(),
Compare.SpecificValue => CompareSpecificValue(_watchList).ToArray(),
Compare.SpecificAddress => CompareSpecificAddress(_watchList).ToArray(),
Compare.Changes => CompareChanges(_watchList).ToArray(),
Compare.Difference => CompareDifference(_watchList).ToArray(),
_ => ComparePrevious(_watchList).ToArray()
};

if (_settings.PreviousType == PreviousType.LastSearch)
{
SetPreviousToCurrent();
}
}

if (UndoEnabled)
{
_history.AddState(_watchList.ToList());
_history.AddState(_watchList.ToArray());
}

return before - _watchList.Count;
return before - _watchList.Length;
}

public bool Preview(long address)
Expand All @@ -164,7 +170,7 @@ public bool Preview(long address)
};
}

public int Count => _watchList.Count;
public int Count => _watchList.Length;

public SearchMode Mode => _settings.Mode;

Expand Down Expand Up @@ -198,12 +204,12 @@ public void Update()
{
if (_settings.IsDetailed())
{
using (_settings.Domain.EnterExit())
{
foreach (IMiniWatchDetails watch in _watchList)
{
watch.Update(_settings.PreviousType, _settings.Domain, _settings.BigEndian);
}
using (_settings.Domain.EnterExit())
{
foreach (IMiniWatchDetails watch in _watchList)
{
watch.Update(_settings.PreviousType, _settings.Domain, _settings.BigEndian);
}
}
}
}
Expand All @@ -225,7 +231,7 @@ public void SetPreviousType(PreviousType type)

public void SetPreviousToCurrent()
{
_watchList.ForEach(w => w.SetPreviousToCurrent(_settings.Domain, _settings.BigEndian));
Array.ForEach(_watchList, w => w.SetPreviousToCurrent(_settings.Domain, _settings.BigEndian));
}

public void ClearChangeCounts()
Expand All @@ -247,7 +253,7 @@ public void RemoveSmallWatchRange(IEnumerable<Watch> watches)
{
if (UndoEnabled)
{
_history.AddState(_watchList.ToList());
_history.AddState(_watchList.ToArray());
}

var addresses = watches.Select(w => w.Address);
Expand All @@ -258,11 +264,11 @@ public void RemoveRange(IEnumerable<int> indices)
{
if (UndoEnabled)
{
_history.AddState(_watchList.ToList());
_history.AddState(_watchList.ToArray());
}

var removeList = indices.Select(i => _watchList[i]); // This will fail after int.MaxValue but RAM Search fails on domains that large anyway
_watchList = _watchList.Except(removeList).ToList();
_watchList = _watchList.Except(removeList).ToArray();
}

public void RemoveAddressRange(IEnumerable<long> addresses)
Expand All @@ -274,7 +280,7 @@ public void AddRange(IEnumerable<long> addresses, bool append)
{
if (!append)
{
_watchList.Clear();
Array.Clear(_watchList, 0, _watchList.Length);
}

var list = _settings.Size switch
Expand All @@ -294,13 +300,13 @@ public void Sort(string column, bool reverse)
switch (column)
{
case WatchList.Address:
_watchList = _watchList.OrderBy(w => w.Address, reverse).ToList();
_watchList = _watchList.OrderBy(w => w.Address, reverse).ToArray();
break;
case WatchList.Value:
_watchList = _watchList.OrderBy(w => GetValue(w.Address), reverse).ToList();
_watchList = _watchList.OrderBy(w => GetValue(w.Address), reverse).ToArray();
break;
case WatchList.Prev:
_watchList = _watchList.OrderBy(w => w.Previous, reverse).ToList();
_watchList = _watchList.OrderBy(w => w.Previous, reverse).ToArray();
break;
case WatchList.ChangesCol:
if (_settings.IsDetailed())
Expand All @@ -309,12 +315,12 @@ public void Sort(string column, bool reverse)
.Cast<IMiniWatchDetails>()
.OrderBy(w => w.ChangeCount, reverse)
.Cast<IMiniWatch>()
.ToList();
.ToArray();
}

break;
case WatchList.Diff:
_watchList = _watchList.OrderBy(w => GetValue(w.Address) - w.Previous, reverse).ToList();
_watchList = _watchList.OrderBy(w => GetValue(w.Address) - w.Previous, reverse).ToArray();
break;
}
}
Expand All @@ -333,26 +339,26 @@ public bool UndoEnabled

public int Undo()
{
int origCount = _watchList.Count;
int origCount = _watchList.Length;
if (UndoEnabled)
{
_watchList = _history.Undo().ToList();
return _watchList.Count - origCount;
_watchList = _history.Undo().ToArray();
return _watchList.Length - origCount;
}

return _watchList.Count;
return _watchList.Length;
}

public int Redo()
{
int origCount = _watchList.Count;
int origCount = _watchList.Length;
if (UndoEnabled)
{
_watchList = _history.Redo().ToList();
return origCount - _watchList.Count;
_watchList = _history.Redo().ToArray();
return origCount - _watchList.Length;
}

return _watchList.Count;
return _watchList.Length;
}

private IEnumerable<IMiniWatch> ComparePrevious(IEnumerable<IMiniWatch> watchList)
Expand Down

0 comments on commit 011f4bf

Please sign in to comment.