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

refactor: compile where/sort/distinct and Advanced Search through expression trees (#2625)

Browse source 
## Summary

`where`, `sort by`, `distinct` and the Advanced Search property test now compile through expression trees instead of the hand-rolled IL in `Emitter.cs`. The emitter and its three `Reflection.Emit` compilers were RunUO-era code from before expression trees existed; they were the only way to avoid per-object reflection at the time, and they are not any more.

Net: ~1,900 lines of IL bookkeeping deleted, one comparison engine instead of two, faster per object, cheaper to compile, collectible, and `Nullable<T>` properties work.

## Why

- **Bugs hid in the IL.** Equality on a type with value semantics but no `IComparable` (`TextDefinition`) was a raw `ceq`, so `where Message = 1060847` never matched. A chained binding (`Message.Number`) dereferenced every link unguarded, so the first swept object with a null intermediate killed the sweep with an NRE. A constant narrower or unsigned than `int` threw before compiling. A struct with no `CompareTo` reached `ceq` on two unboxed values, which is invalid IL. Every dynamic assembly was `Run`, so each `[global where` grew the process for good.
- **Advanced Search had its own engine.** Per entity and per leaf it re-split the expression, scanned the runtime type's properties by name, read the value by reflection, re-parsed the right-hand side and dispatched on type through ~300 lines of `CompareValues`. Same job, second implementation, second set of bugs.

## What changed

- `ICondition.Compile(MethodEmitter)` becomes `ICondition.Build(ParameterExpression)` returning an `Expression`. `ConditionalCompiler` assembles a `Func<object, bool>`, `SortCompiler` a `Comparison<T>`, `DistinctCompiler` both comparer interfaces over one lambda. The parsed constant is an `Expression.Constant`, so the generated type, its constructor and the per-condition field for non-primitive constants disappear with `PropertyValue`.
- `PropertyExpressions` holds the shared pieces: the chain walk with its null-intermediate guard, `CompareTo` resolution with the old null ordering, the integral/enum operator path, and constant parsing.
- `BaseExtension.Optimize` loses its `ref AssemblyEmitter` parameter. An out-of-tree extension that overrides `Optimize` needs to drop it.
- Advanced Search keeps its grammar (`~` negates, `@` is AND, `|` is OR and binds looser, string `>` is "starts with") and translates each leaf into the same conditions, compiled once per declaring type per search and memoized across the workers. Float and double keep their typed-precision tolerance through a small `EpsilonCondition` in the Advanced Search folder.

## Semantics preserved

- Equality on a non-comparable reference type is `object.Equals`, never reference identity. A non-comparable struct boxes into the same call.
- A null intermediate in a chained binding is no match, and stays no match under negation. Sort and distinct read it as `default(T)`.
- Unsigned relational compares stay unsigned; integral primitives and enums use the operator directly, nothing widens to a signed type. `float`, `double`, `decimal`, `string` and structs still go through the type's own `CompareTo`, so `string` equality stays culture-sensitive exactly as before.
- Only `==` and `!=` are valid for non-comparable types; a relational operator still throws at build time.
- `TypeCondition` is still first and still null-checks the cast target.

## Behavior changes

- **`Nullable<T>` works**, with C# lifted semantics in `where`: two nulls are equal, a null and a value are unequal, a null satisfies no relation. Sort keeps a total order with unset values at one end. A null *reference* keeps the ordering it had.
- **Advanced Search**: a leaf that cannot be parsed or resolved is no match even under `~` (it used to negate the failure and match every entity); `null` is the null value for equality on strings, nullables and reference types, as in `where`; dotted names walk into a property; static properties are no longer searchable.

## Measurements

`where`, from the handoff (Debug test host, single condition, ratios not absolutes):

| Approach | ns/object | Compile | Collectible | LOC |
|---|---:|---:|---|---:|
| `AssemblyBuilder` + IL (before) | 20.5 | 0.311 ms | no (`Run`) | ~1,916 |
| Expression trees (after) | ~12 | 0.187 ms warm | yes | ~600 |

Advanced Search, Release, one `SkillTeleporter`, 2M evaluations per leaf:

| Leaf | Before | After |
|---|---:|---:|
| `Hue=5` | 137 ns | 20 ns |
| `Name~~gate` | 121 ns | 39 ns |
| `Skill=Magery` | 116 ns | 21 ns |
| `Weight>0.5` | 122 ns | 28 ns |

Plus 0.5 to 2 ms to compile each declaring type a search meets (12 ms for the first compile in the process). All of it runs on the search workers; nothing new touches the loop.

## Also fixed: Advanced Search map filters

Found while testing the property test in game. The map boxes are independent checkboxes, but the worker applied each ticked map as "must be on this map", so ticking two or more (all maps and Internal, say) rejected every entity before any other filter ran. Present since #1649; the default of Felucca alone never showed it. An entity now passes when its map is any of the ticked ones, with none ticked meaning no map constraint. Pinned by a worker test.

## Test plan

- [x] `UOContent.Tests`: 862 passed, 0 failed
- [x] `Server.Tests`: 869 passed, 0 failed
- [x] Every commit builds and its tests pass on its own (bisectable)
- [ ] In game: `[global where`, `[area where`, `[condition`, `sort by`, `distinct`, Advanced Search property test
This commit is contained in:
Kamron Batman 2026-09-10 19:01:59 -07:00 • committed by GitHub
parent 865a8cf218
commit 535a098996
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 2294 additions and 2142 deletions
Download patch file Download diff file Expand all files Collapse all files

379
Projects/UOContent/Engines/Advanced Search/AdvancedSearchUtilities.cs
View file

@ -1,8 +1,5 @@
using System;
using System.Buffers;
using System.Globalization;
using System.Numerics;
using System.Runtime.CompilerServices;
namespace Server.Engines.AdvancedSearch;
@ -36,139 +33,6 @@ public static class AdvancedSearchUtilities
return expression.Slice(index, 1);
}
public static bool CompareValues(Type propertyType, object propertyValue, ReadOnlySpan<char> valuePart, ReadOnlySpan<char> operatorSpan)
{
// TODO: Add support for implicit conversion types like Serial -> uint
if (propertyType == typeof(long))
{
return TryParseValue<long>(valuePart, out var parsedValue) &&
CompareNumeric((long)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(ulong))
{
return TryParseValue<ulong>(valuePart, out var parsedValue) &&
CompareNumeric((ulong)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(int))
{
return TryParseValue<int>(valuePart, out var parsedValue) &&
CompareNumeric((int)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(uint))
{
return TryParseValue<uint>(valuePart, out var parsedValue) &&
CompareNumeric((uint)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(short))
{
return TryParseValue<short>(valuePart, out var parsedValue) &&
CompareNumeric((short)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(ushort))
{
return TryParseValue<ushort>(valuePart, out var parsedValue) &&
CompareNumeric((ushort)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(sbyte))
{
return TryParseValue<sbyte>(valuePart, out var parsedValue) &&
CompareNumeric((sbyte)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(byte))
{
return TryParseValue<byte>(valuePart, out var parsedValue) &&
CompareNumeric((byte)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(float))
{
return TryParseValue<float>(valuePart, out var parsedValue) &&
Compare((float)propertyValue!, parsedValue, valuePart, operatorSpan);
}
if (propertyType == typeof(double))
{
return TryParseValue<double>(valuePart, out var parsedValue) &&
Compare((double)propertyValue!, parsedValue, valuePart, operatorSpan);
}
if (propertyType == typeof(string))
{
return TryParseValue<string>(valuePart, out var parsedValue) &&
Compare((string)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(TimeSpan))
{
return TryParseValue<TimeSpan>(valuePart, out var parsedValue) &&
Compare((TimeSpan)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(DateTime))
{
return TryParseValue<DateTime>(valuePart, out var parsedValue) &&
Compare((DateTime)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType == typeof(bool))
{
return TryParseValue<bool>(valuePart, out var parsedValue) &&
Compare((bool)propertyValue!, parsedValue, operatorSpan);
}
if (propertyType.IsEnum)
{
if (!Enum.TryParse(propertyType, valuePart.ToString(), true, out var valueEnum) || valueEnum == null)
{
return false;
}
return GetEnumSize(propertyType) switch
{
1 => CompareNumeric((byte)propertyValue!, (byte)valueEnum, operatorSpan),
2 => CompareNumeric((short)propertyValue!, (short)valueEnum, operatorSpan),
4 => CompareNumeric((int)propertyValue!, (int)valueEnum, operatorSpan),
8 => CompareNumeric((long)propertyValue!, (long)valueEnum, operatorSpan),
_ => false
};
}
// Anything the hot typed paths above didn't handle — reference types (Poison, Map, entity
// properties resolved by serial), IParsable value types (Guid, decimal, ...), and legacy
// RunUO types with a static Parse(string) (Faction, Town, ...). Delegate to the shared,
// thread-safe Types converter so the target is parsed into the property's real type, then
// compare by value. A string is allocated here, but this is the uncommon path; the common
// types never reach it. Types returns a non-null message when it can't parse -> no match.
return Types.TryParse(propertyType, valuePart.ToString(), out var parsed) == null &&
CompareReference(propertyValue!, parsed, operatorSpan);
}
public static bool CompareNumeric<T>(T propertyValue, T parsedValue, ReadOnlySpan<char> operatorSpan) where T : INumber<T> =>
operatorSpan switch
{
"=" or "==" => propertyValue == parsedValue,
"!" or "!=" => propertyValue != parsedValue,
">" => propertyValue > parsedValue,
"<" => propertyValue < parsedValue,
">=" => propertyValue >= parsedValue,
"<=" => propertyValue <= parsedValue,
_ => false
};
public static bool Compare(
double propertyValue,
double parsedValue,
ReadOnlySpan<char> originalValue,
ReadOnlySpan<char> operatorSpan
)
{
var epsilon = CalculateEpsilon(originalValue);
return operatorSpan switch
{
"=" or "==" => Math.Abs(propertyValue - parsedValue) < epsilon,
"!" or "!=" => Math.Abs(propertyValue - parsedValue) >= epsilon,
">" => propertyValue > parsedValue + epsilon,
"<" => propertyValue < parsedValue - epsilon,
">=" => propertyValue >= parsedValue - epsilon,
"<=" => propertyValue <= parsedValue + epsilon,
_ => throw new ArgumentException("Invalid operator")
};
}
public static double CalculateEpsilon(ReadOnlySpan<char> value)
{
var decimalPlace = value.IndexOf('.');
@ -191,247 +55,4 @@ public static class AdvancedSearchUtilities
_ => 1E-16
};
}
public static bool Compare(string propertyValue, string parsedValue, ReadOnlySpan<char> operatorSpan) =>
operatorSpan switch
{
"=" or "==" => propertyValue.EqualsOrdinal(parsedValue),
"!" or "!=" => !propertyValue.EqualsOrdinal(parsedValue),
">" => propertyValue.StartsWithOrdinal(parsedValue),
"<" => propertyValue.EndsWithOrdinal(parsedValue),
"~" => propertyValue.Contains(parsedValue),
"~<" => propertyValue.InsensitiveEndsWith(parsedValue),
"~>" => propertyValue.InsensitiveStartsWith(parsedValue),
"~~" => propertyValue.InsensitiveContains(parsedValue),
"~=" => propertyValue.InsensitiveEquals(parsedValue),
"~!" => !propertyValue.InsensitiveEquals(parsedValue),
_ => false
};
public static bool Compare(TimeSpan propertyValue, TimeSpan parsedValue, ReadOnlySpan<char> operatorSpan) =>
operatorSpan switch
{
"=" or "==" => propertyValue == parsedValue,
"!" or "!=" => propertyValue != parsedValue,
">" => propertyValue > parsedValue,
"<" => propertyValue < parsedValue,
">=" => propertyValue >= parsedValue,
"<=" => propertyValue <= parsedValue,
_ => false
};
public static bool Compare(DateTime propertyValue, DateTime parsedValue, ReadOnlySpan<char> operatorSpan) =>
operatorSpan switch
{
"=" or "==" => propertyValue == parsedValue,
"!" or "!=" => propertyValue != parsedValue,
">" => propertyValue > parsedValue,
"<" => propertyValue < parsedValue,
">=" => propertyValue >= parsedValue,
"<=" => propertyValue <= parsedValue,
_ => false
};
public static bool Compare(bool propertyValue, bool parsedValue, ReadOnlySpan<char> operatorSpan) =>
operatorSpan switch
{
"=" or "==" => propertyValue == parsedValue,
"!" or "!=" => propertyValue != parsedValue,
_ => false
};
public static bool CompareReference<T>(T propertyValue, T parsedValue, ReadOnlySpan<char> operatorSpan)
{
switch (operatorSpan)
{
case "=":
case "==": return Equals(propertyValue, parsedValue);
case "!":
case "!=": return !Equals(propertyValue, parsedValue);
}
if (propertyValue is IComparable cmp && parsedValue != null)
{
try
{
var c = cmp.CompareTo(parsedValue);
return operatorSpan switch
{
">" => c > 0,
"<" => c < 0,
">=" => c >= 0,
"<=" => c <= 0,
_ => false
};
}
catch
{
return false;
}
}
return false;
}
internal static bool TryParseValue<T>(ReadOnlySpan<char> valuePart, out T value)
{
// Special handling for boolean and hexadecimal values
if (typeof(T) == typeof(bool))
{
var val = valuePart.ToString().ToLower();
if (val is "true" or "1" or "enabled" or "on")
{
value = (T)(object)true;
return true;
}
if (val is "false" or "0" or "disabled" or "off")
{
value = (T)(object)false;
return true;
}
value = default;
return false;
}
if (typeof(T) == typeof(long))
{
return TryParseNumericValue<long, T>(valuePart, out value);
}
if (typeof(T) == typeof(ulong))
{
return TryParseNumericValue<ulong, T>(valuePart, out value);
}
if (typeof(T) == typeof(int))
{
return TryParseNumericValue<int, T>(valuePart, out value);
}
if (typeof(T) == typeof(uint))
{
return TryParseNumericValue<uint, T>(valuePart, out value);
}
if (typeof(T) == typeof(short))
{
return TryParseNumericValue<short, T>(valuePart, out value);
}
if (typeof(T) == typeof(ushort))
{
return TryParseNumericValue<ushort, T>(valuePart, out value);
}
if (typeof(T) == typeof(sbyte))
{
return TryParseNumericValue<sbyte, T>(valuePart, out value);
}
if (typeof(T) == typeof(byte))
{
return TryParseNumericValue<byte, T>(valuePart, out value);
}
if (typeof(T) == typeof(float))
{
return TryParseNumericValue<float, T>(valuePart, out value);
}
if (typeof(T) == typeof(double))
{
return TryParseNumericValue<double, T>(valuePart, out value);
}
// string needs no parsing — the span itself is the value.
if (typeof(T) == typeof(string))
{
value = (T)(object)valuePart.ToString();
return true;
}
// Remaining supported types (TimeSpan, DateTime) parse straight from the span via
// ISpanParsable<T> — no allocation, no reflection, and unlike Convert.ChangeType it handles
// TimeSpan, which is not IConvertible and previously failed silently.
if (typeof(T) == typeof(TimeSpan))
{
return TryParseSpanParsable<TimeSpan, T>(valuePart, out value);
}
if (typeof(T) == typeof(DateTime))
{
return TryParseSpanParsable<DateTime, T>(valuePart, out value);
}
value = default;
return false;
}
// Parses U (a value type exposing ISpanParsable<U>) from the span and reinterprets it as T. The
// two type params mirror TryParseNumericValue: the caller dispatches on typeof(T), so U == T at
// every call site and the (T)(object) cast is always valid.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool TryParseSpanParsable<U, T>(ReadOnlySpan<char> valuePart, out T value) where U : ISpanParsable<U>
{
if (U.TryParse(valuePart, null, out var parsed))
{
value = (T)(object)parsed;
return true;
}
value = default;
return false;
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool TryParseNumericValue<T, R>(ReadOnlySpan<char> valuePart, out R value) where T : INumber<T>
{
var ok = valuePart.StartsWith("0x")
? T.TryParse(valuePart[2..], NumberStyles.HexNumber, null, out var parsed)
: T.TryParse(valuePart, null, out parsed);
if (ok)
{
value = (R)(object)parsed;
return true;
}
value = default;
return false;
}
// Evaluates one trimmed leaf atom against caller-supplied state. A custom delegate is required
// because ReadOnlySpan<char> cannot be a Func<> type argument; passing state avoids a per-call
// capturing closure, so the recursion allocates neither a string nor a closure.
internal delegate bool LeafEvaluator<in TState>(TState state, ReadOnlySpan<char> leaf);
// OR ('|') binds looser than AND ('@'); split on the outermost OR first, then AND.
internal static bool EvaluateBoolean<TState>(ReadOnlySpan<char> expr, TState state, LeafEvaluator<TState> evalLeaf)
{
var orIndex = expr.IndexOf('|');
if (orIndex != -1)
{
return EvaluateBoolean(expr[..orIndex], state, evalLeaf) || EvaluateBoolean(expr[(orIndex + 1)..], state, evalLeaf);
}
var andIndex = expr.IndexOf('@');
if (andIndex != -1)
{
return EvaluateBoolean(expr[..andIndex], state, evalLeaf) && EvaluateBoolean(expr[(andIndex + 1)..], state, evalLeaf);
}
return evalLeaf(state, expr.Trim());
}
private static int GetEnumSize(Type enumType) =>
Type.GetTypeCode(Enum.GetUnderlyingType(enumType)) switch
{
TypeCode.Byte or TypeCode.SByte => sizeof(byte),
TypeCode.Int16 or TypeCode.UInt16 => sizeof(ushort),
TypeCode.Int32 or TypeCode.UInt32 => sizeof(uint),
TypeCode.Int64 or TypeCode.UInt64 => sizeof(ulong),
_ => 4
};
}