warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: Optimizes FindItemsByType by removing allocations. (#1515)

Browse source 
### Summary
Container enumeration is in dire need of optimization. Thanks to @stefanomerotta for initiating this work with PR #1443. This PR handles a small part of what Stefan started. Also included are some bug fixes.

### Method Signatures

```cs
// Use with foreach without moving/deleting items
FindItemsByTypeEnumerator<T> FindItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)

// Use with foreach when moving/deleting items
QueuedItemsEnumerator<T> EnumerateItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)

// Use when iterating multiple times or queuing
PooledRefQueue<T> QueueItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)

// Use when iterating multiples times or manipulating elements without traversing
PooledRefList<T> ListItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
```

* `FindItemsByType<T>` has changed from returning `List<T>` to `FindItemsByTypeEnumerator<T>` - This method is not safe to use in situations where an item may get consumed, deleted, or moved.

* `EnumerateItemsByType<T>` was added as a safe way to iterate and manipulate items.
  * **Note**: EnumerateItemsByType will _completely traverse the container_ before iteration starts because it uses `QueueItemsByType` under the hood.

* `QueueItemsByType<T>`  and `ListItemsByType<T>` was added to return a queue or list of items to iterate multiple times and manipulate the items. This isn't the most efficient since it uses a predicate and can result in 2 or 3 total iterations unnecessarily.

### Bug Fixes
- [X] Fishing had an error in the random check that may have caused slight bias.
This commit is contained in:
Kamron Batman 2023-09-28 19:36:45 -07:00 • committed by GitHub
parent 146abad36e
commit a4cabe2fa4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 655 additions and 390 deletions
Download patch file Download diff file Expand all files Collapse all files

243
Projects/Server/Items/Container.Enumerable.cs Normal file
View file

@ -0,0 +1,243 @@
/*************************************************************************
* ModernUO *
* Copyright 2019-2023 - ModernUO Development Team *
* Email: hi@modernuo.com *
* File: Container.Enumerable.cs *
* *
* This program is free software: you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation, either version 3 of the License, or *
* (at your option) any later version. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Server.Collections;
namespace Server.Items;
public partial class Container
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public FindItemsByTypeEnumerator<Item> FindItemsByType(bool recurse = true, Predicate<Item> predicate = null)
=> FindItemsByType<Item>(recurse, predicate);
/// <summary>
/// Performs a breadth-first search through all the <see cref="Item" />s and
/// nested <see cref="Container" />s within this <see cref="Container" />.
/// </summary>
/// <remarks>
/// DO NOT consume, delete, or move items while iterating
/// </remarks>
/// <example>
/// <code>
/// var total = 0;
/// foreach (var gold in cont.FindItemsByType&lt;Gold&gt;())
/// {
/// total += gold.Amount;
/// }
/// </code>
/// </example>
/// <typeparam name="T">Type of objects being searched for</typeparam>
/// <param name="recurse">
/// Optional: If true, the search will recursively
/// check any nested <see cref="Container" />s; otherwise, nested
/// <see cref="Container" />s will not be searched.
/// </param>
/// <param name="predicate">
/// Optional: A predicate to check if the <see cref="Item" />
/// of type <typeparamref name="T" /> is one of the targets of the search.
/// </param>
/// <returns>
/// An enumerator for iterating through <see cref="Item" />s of type <typeparamref name="T" /> that match the optional
/// <paramref name="predicate" />.
/// </returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public FindItemsByTypeEnumerator<T> FindItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
where T : Item => new(this, recurse, predicate);
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public QueuedItemsEnumerator<Item> EnumerateItemsByType(bool recurse = true, Predicate<Item> predicate = null)
=> EnumerateItemsByType<Item>(recurse, predicate);
/// <summary>
/// Safely enumerates items using a breadth-first search through all the <see cref="Item" />s and
/// nested <see cref="Container" />s within this <see cref="Container" />.
/// </summary>
/// <remarks>
/// Use EnumerateItemsByType for situations where the item might be manipulated, consumed, or moved.
/// Note: This method scans through the container before returning the enumerator for iteration.
/// </remarks>
/// <example>
/// <code>
/// foreach (var item in cont.EnumerateItemsByType&lt;Item&gt;())
/// {
/// if (item.LootType is not LootType.Blessed)
/// {
/// item.Delete();
/// }
/// }
/// </code>
/// </example>
/// <typeparam name="T">Type of objects being searched for</typeparam>
/// <param name="recurse">
/// Optional: If true, the search will recursively
/// check any nested <see cref="Container" />s; otherwise, nested
/// <see cref="Container" />s will not be searched.
/// </param>
/// <param name="predicate">
/// Optional: A predicate to check if the <see cref="Item" />
/// of type <typeparamref name="T" /> is one of the targets of the search.
/// </param>
/// <returns>
/// An enumerator for iterating through <see cref="Item" />s of type <typeparamref name="T" /> that match the optional
/// <paramref name="predicate" />.
/// </returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public QueuedItemsEnumerator<T> EnumerateItemsByType<T>(bool recurse = true, Predicate<T> predicate = null)
where T : Item => new(QueueItemsByType(recurse, predicate));
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public PooledRefQueue<Item> QueueItemsByType(bool recurse = true, Predicate<Item> predicate = null) =>
QueueItemsByType<Item>(recurse, predicate);
public PooledRefQueue<T> QueueItemsByType<T>(bool recurse = true, Predicate<T> predicate = null) where T : Item
{
var queue = PooledRefQueue<T>.Create();
foreach (var item in FindItemsByType(recurse, predicate))
{
queue.Enqueue(item);
}
return queue;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public PooledRefList<Item> ListItemsByType(bool recurse = true, Predicate<Item> predicate = null) =>
ListItemsByType<Item>(recurse, predicate);
public PooledRefList<T> ListItemsByType<T>(bool recurse = true, Predicate<T> predicate = null) where T : Item
{
var list = PooledRefList<T>.Create();
foreach (var item in FindItemsByType(recurse, predicate))
{
list.Add(item);
}
return list;
}
public ref struct FindItemsByTypeEnumerator<T> where T : Item
{
private PooledRefQueue<Container> _containers;
private Span<Item> _items;
private int _index;
private T _current;
private bool _recurse;
private Predicate<T> _predicate;
public FindItemsByTypeEnumerator(Container container, bool recurse, Predicate<T> predicate)
{
_containers = PooledRefQueue<Container>.Create();
if (container?.m_Items != null)
{
_items = CollectionsMarshal.AsSpan(container.m_Items);
}
_current = default;
_index = 0;
_recurse = recurse;
_predicate = predicate;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool MoveNext() => SetNextItem() || _recurse && SetNextContainer();
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool SetNextContainer()
{
if (!_containers.TryDequeue(out var c))
{
return false;
}
_items = CollectionsMarshal.AsSpan(c.m_Items);
_index = 0;
return SetNextItem();
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool SetNextItem()
{
while (_index < _items.Length)
{
Item item = _items[_index++];
if (_recurse && item is Container { m_Items.Count: > 0 } c)
{
_containers.Enqueue(c);
}
if (item is T t && _predicate?.Invoke(t) != false)
{
_current = t;
return true;
}
}
return false;
}
public T Current
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => _current;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose() => _containers.Dispose();
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public FindItemsByTypeEnumerator<T> GetEnumerator() => this;
}
public ref struct QueuedItemsEnumerator<T> where T : Item
{
private PooledRefQueue<T> _queue;
private T _current;
public QueuedItemsEnumerator(PooledRefQueue<T> queue)
{
_queue = queue;
_current = default;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool MoveNext()
{
if (_queue.TryDequeue(out var item))
{
_current = item;
return true;
}
return false;
}
public T Current
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => _current;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose() => _queue.Dispose();
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public QueuedItemsEnumerator<T> GetEnumerator() => this;
}
}

223
Projects/Server/Items/Container.cs
View file

@ -14,10 +14,8 @@ public delegate int CheckItemGroup(Item a, Item b);
public delegate void ContainerSnoopHandler(Container cont, Mobile from);
public class Container : Item
public partial class Container : Item
{
private static readonly List<Item> m_FindItemsList = new();
private ContainerData m_ContainerData;
private int m_DropSound;
@ -817,19 +815,26 @@ public class Container : Item
throw new ArgumentNullException(nameof(grouper));
}
var typedItems = CollectionsMarshal.AsSpan(FindItemsByType(type, recurse));
using var typedItems = PooledRefList<Item>.Create();
foreach (var item in FindItemsByType(recurse))
{
if (type.IsInstanceOfType(item))
{
typedItems.Add(item);
}
}
var groups = new List<List<Item>>();
var idx = 0;
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var a = typedItems[idx++];
var group = new List<Item>();
group.Add(a);
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var b = typedItems[idx];
var v = grouper(a, b);
@ -929,19 +934,27 @@ public class Container : Item
for (var i = 0; i < types.Length; ++i)
{
var typedItems = CollectionsMarshal.AsSpan(FindItemsByType(types[i], recurse));
var type = types[i];
using var typedItems = PooledRefList<Item>.Create();
foreach (var item in FindItemsByType(recurse))
{
if (type.IsInstanceOfType(item))
{
typedItems.Add(item);
}
}
var groups = new List<List<Item>>();
var idx = 0;
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var a = typedItems[idx++];
var group = new List<Item>();
group.Add(a);
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var b = typedItems[idx];
var v = grouper(a, b);
@ -1253,25 +1266,31 @@ public class Container : Item
public bool ConsumeTotal(Type type, int amount = 1, bool recurse = true, OnItemConsumed callback = null)
{
var items = CollectionsMarshal.AsSpan(FindItemsByType(type, recurse));
var total = 0;
using var items = PooledRefQueue<Item>.Create();
// First pass, compute total
var total = 0;
for (var i = 0; i < items.Length; ++i)
foreach (var item in FindItemsByType(recurse))
{
total += items[i].Amount;
if (type.IsInstanceOfType(item))
{
Items.Add(item);
total += item.Amount;
if (total >= amount)
{
break;
}
}
}
// We have enough, so consume it
if (total >= amount)
{
// We've enough, so consume it
var need = amount;
for (var i = 0; i < items.Length; ++i)
while (items.Count > 0)
{
var item = items[i];
var item = items.Dequeue();
var theirAmount = item.Amount;
@ -1287,7 +1306,6 @@ public class Container : Item
callback?.Invoke(item, need);
item.Consume(need);
return true;
}
}
@ -1361,12 +1379,19 @@ public class Container : Item
var best = 0;
var typedItems = CollectionsMarshal.AsSpan(FindItemsByType(type, recurse));
using var typedItems = PooledRefList<Item>.Create();
foreach (var item in FindItemsByType(recurse))
{
if (type.IsInstanceOfType(item))
{
typedItems.Add(item);
}
}
var groups = new List<List<Item>>();
var idx = 0;
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var a = typedItems[idx++];
var group = new List<Item>
@ -1374,7 +1399,7 @@ public class Container : Item
a
};
while (idx < typedItems.Length)
while (idx < typedItems.Count)
{
var b = typedItems[idx];
var v = grouper(a, b);
@ -1540,9 +1565,12 @@ public class Container : Item
public int GetAmount(Type type, bool recurse = true)
{
var total = 0;
foreach (var item in FindItemsByType(type, recurse))
foreach (var item in FindItemsByType(recurse))
{
total += item.Amount;
if (type.IsInstanceOfType(item))
{
total += item.Amount;
}
}
return total;
@ -1551,9 +1579,12 @@ public class Container : Item
public int GetAmount(Type[] types, bool recurse = true)
{
var total = 0;
foreach (var item in FindItemsByType(types, recurse))
foreach (var item in FindItemsByType(recurse))
{
total += item.Amount;
if (InTypeList(item, types))
{
total += item.Amount;
}
}
return total;
@ -1561,23 +1592,12 @@ public class Container : Item
public List<Item> FindItemsByType(Type type, bool recurse = true)
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
var items = new List<Item>();
while (queue.Count > 0)
foreach (var item in FindItemsByType(recurse))
{
var container = queue.Dequeue();
foreach (var item in container.Items)
if (type.IsInstanceOfType(item))
{
if (type.IsInstanceOfType(item))
{
items.Add(item);
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
items.Add(item);
}
}
@ -1586,23 +1606,12 @@ public class Container : Item
public List<Item> FindItemsByType(Type[] types, bool recurse = true)
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
var items = new List<Item>();
while (queue.Count > 0)
foreach (var item in FindItemsByType(recurse))
{
var container = queue.Dequeue();
foreach (var item in container.Items)
if (InTypeList(item, types))
{
if (InTypeList(item, types))
{
items.Add(item);
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
items.Add(item);
}
}
@ -1611,22 +1620,11 @@ public class Container : Item
public Item FindItemByType(Type type, bool recurse = true)
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
while (queue.Count > 0)
foreach (var item in FindItemsByType(recurse))
{
var container = queue.Dequeue();
foreach (var item in container.Items)
if (type.IsInstanceOfType(item))
{
if (type.IsInstanceOfType(item))
{
return item;
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
return item;
}
}
@ -1635,73 +1633,17 @@ public class Container : Item
public Item FindItemByType(Type[] types, bool recurse = true)
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
while (queue.Count > 0)
foreach (var item in FindItemsByType(recurse))
{
var container = queue.Dequeue();
foreach (var item in container.Items)
if (InTypeList(item, types))
{
if (InTypeList(item, types))
{
return item;
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
return item;
}
}
return null;
}
public List<T> FindItemsByType<T>(Predicate<T> predicate) where T : Item => FindItemsByType(true, predicate);
/// <summary>
/// Performs a Breadth-First search through all the <see cref="Item" />s and
/// nested <see cref="Container" />s within this <see cref="Container" />.
/// </summary>
/// <typeparam name="T">Type of objects being searched for</typeparam>
/// <param name="recurse">
/// Optional: If true, the search will recursively
/// check any nested <see cref="Container" />s; otherwise, nested
/// <see cref="Container" />s will not be searched.
/// </param>
/// <param name="predicate">
/// Optional: A predicate to check if the <see cref="Item" />
/// of type <typeparamref name="T" /> is one of the targets of the search.
/// </param>
/// <returns>
/// A list of <see cref="Item" />s of type <typeparamref name="T" /> that matche the optional
/// <paramref name="predicate" />.
/// </returns>
public List<T> FindItemsByType<T>(bool recurse = true, Predicate<T> predicate = null) where T : Item
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
var items = new List<T>();
while (queue.Count > 0)
{
var container = queue.Dequeue();
foreach (var item in container.Items)
{
if (item is T typedItem && predicate?.Invoke(typedItem) != false)
{
items.Add(typedItem);
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
}
}
return items;
}
/// <summary>
/// Performs a Breadth-First search through all the <see cref="Item" />s and
/// nested <see cref="Container" />s within this <see cref="Container" />.
@ -1722,37 +1664,14 @@ public class Container : Item
/// </returns>
public T FindItemByType<T>(bool recurse = true, Predicate<T> predicate = null) where T : Item
{
using var queue = PooledRefQueue<Container>.Create(128);
queue.Enqueue(this);
while (queue.Count > 0)
foreach (var item in FindItemsByType(recurse, predicate))
{
var container = queue.Dequeue();
foreach (var item in container.Items)
{
if (item is T typedItem && predicate?.Invoke(typedItem) != false)
{
return typedItem;
}
if (recurse && item is Container itemContainer)
{
queue.Enqueue(itemContainer);
}
}
return item;
}
return null;
}
private class GroupComparer : IComparer<Item>
{
private readonly CheckItemGroup m_Grouper;
public GroupComparer(CheckItemGroup grouper) => m_Grouper = grouper;
public int Compare(Item a, Item b) => m_Grouper(a, b);
}
[Flags]
private enum SaveFlag : byte
{

2
Projects/Server/Mobiles/IMount.cs
View file

@ -13,8 +13,6 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System;
namespace Server.Mobiles;
public interface IMount : IHasSteps