diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs index 4224573..8539640 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Conversion/MxStatusProxyConverterTests.cs @@ -135,6 +135,90 @@ public sealed class MxStatusProxyConverterTests Assert.Equal("Invalid reference", second.DiagnosticText); } + /// + /// Verifies every accessor strategy converts the same status values + /// identically. PERF-20 reads a public field of a publicly visible type + /// through a delegate compiled from an expression tree instead of + /// FieldInfo.GetValue; a field type whose conversion to int is not + /// a lossless widening () and a type the + /// compiled delegate may not touch (, not + /// visible outside this assembly) keep the reflection read. All four + /// doubles below carry the same logical status, so all four messages must + /// be equal. + /// + [Fact] + public void Convert_AcrossAccessorStrategies_ProducesIdenticalMessages() + { + // Struct, widening field types: the compiled accessor unboxes in place. + MxStatusProxy compiledStruct = _converter.Convert(new FakeMxStatusProxy + { + success = 1, + category = 5, + detectedBy = 3, + detail = 21, + }); + + // Reference type, widening field types: the compiled accessor casts. + MxStatusProxy compiledClass = _converter.Convert(new FakeStatusProxyClass + { + success = 1, + category = 5, + detectedBy = 3, + detail = 21, + }); + + // Non-widening field types: reflection + Convert.ToInt32 is kept. + MxStatusProxy reflectedWide = _converter.Convert(new FakeWideStatusProxy + { + success = 1L, + category = 5L, + detectedBy = 3L, + detail = 21L, + }); + + // Type not visible outside the assembly: reflection is kept. + MxStatusProxy reflectedHidden = _converter.Convert(new HiddenStatusProxy + { + success = 1, + category = 5, + detectedBy = 3, + detail = 21, + }); + + Assert.Equal(compiledStruct, compiledClass); + Assert.Equal(compiledStruct, reflectedWide); + Assert.Equal(compiledStruct, reflectedHidden); + Assert.Equal(MxStatusCategory.OperationalError, compiledStruct.Category); + Assert.Equal(MxStatusSource.RespondingNmx, compiledStruct.DetectedBy); + Assert.Equal("Invalid reference", compiledStruct.DiagnosticText); + } + + /// + /// Verifies the compiled accessor widens a negative 16-bit field the way + /// Convert.ToInt32 did — sign-extended, not reinterpreted. The + /// interop MXSTATUS_PROXY declares 16-bit fields, so a sign-extension + /// mistake here would silently change every failing status the gateway + /// reports. + /// + [Fact] + public void Convert_WithExtremeInt16FieldValues_SignExtendsLikeConvertToInt32() + { + FakeMxStatusProxy status = new() + { + success = short.MinValue, + category = 2, + detectedBy = 0, + detail = short.MaxValue, + }; + + MxStatusProxy converted = _converter.Convert(status); + + Assert.Equal((int)short.MinValue, converted.Success); + Assert.Equal((int)short.MaxValue, converted.Detail); + Assert.Equal(MxStatusCategory.Warning, converted.Category); + Assert.Equal(MxStatusSource.RequestingLmx, converted.DetectedBy); + } + public struct FakeMxStatusProxy { public short success; @@ -146,6 +230,39 @@ public sealed class MxStatusProxyConverterTests public short detail; } + public struct FakeWideStatusProxy + { + public long success; + + public long category; + + public long detectedBy; + + public long detail; + } + + public sealed class FakeStatusProxyClass + { + public int success; + + public int category; + + public int detectedBy; + + public int detail; + } + + private struct HiddenStatusProxy + { + public short success; + + public int category; + + public int detectedBy; + + public short detail; + } + private sealed class MissingFields { } diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs index 9e1c09d..fba52ec 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessEventMapperTests.cs @@ -238,6 +238,97 @@ public sealed class MxAccessEventMapperTests Assert.False(MxAccessEventMapper.TryParseSourceTimestamp(text, out _)); } + /// + /// Verifies the exact-format fast path PERF-20 added in front of the + /// general parse chain returns exactly what that chain returned. The + /// cases cover the captured MXAccess shape (which the fast path is meant + /// to catch on a US-shaped host), its zero-padded and second-precision + /// siblings, and strings the derived formats cannot match — an ISO + /// round-trip form, a long-date form, and a day-first European form — + /// which must fall through to the unchanged two-stage chain. The + /// expectation is computed with the pre-PERF-20 chain in this test, so it + /// holds on any host culture: the fast path is a shortcut, never a + /// different answer. + /// + /// Timestamp string to parse. + [Theory] + [InlineData("3/26/2026 1:38:22.907 PM")] + [InlineData("3/26/2026 1:38:22 PM")] + [InlineData("03/26/2026 01:38:22 PM")] + [InlineData("12/31/2026 11:59:59.999 PM")] + [InlineData("2026-03-26T13:38:22.9070000")] + [InlineData("Thursday, March 26, 2026 1:38:22 PM")] + [InlineData("26.03.2026 13:38:22")] + [InlineData("not a timestamp")] + public void TryParseSourceTimestamp_MatchesPreFastPathChain(string text) + { + bool expectedParsed = TryParseWithGeneralChainOnly(text, out DateTime expectedUtc); + + bool parsed = MxAccessEventMapper.TryParseSourceTimestamp(text, out DateTime utc); + + Assert.Equal(expectedParsed, parsed); + Assert.Equal(expectedUtc, utc); + if (parsed) + { + Assert.Equal(DateTimeKind.Utc, utc.Kind); + } + } + + /// + /// Verifies a timestamp written in the host culture's own patterns — + /// including the millisecond fraction MXAccess appends, which no culture + /// publishes in its long-time pattern — parses as local wall-clock time + /// and comes back as UTC. This is the string shape the derived exact + /// formats are built for, so it exercises the fast path on the worker's + /// own host regardless of which culture that host runs. + /// + [Fact] + public void TryParseSourceTimestamp_WithCultureShapedMillisecondTimestamp_ReturnsLocalWallClockAsUtc() + { + CultureInfo culture = CultureInfo.CurrentCulture; + string longTime = culture.DateTimeFormat.LongTimePattern; + int secondsIndex = longTime.IndexOf("ss", StringComparison.Ordinal); + + // Every Windows culture publishes seconds in its long-time pattern; the + // guard keeps the test honest rather than green-by-accident if one does not. + Assert.True(secondsIndex >= 0, $"Long-time pattern '{longTime}' has no seconds specifier."); + + DateTime localWall = new(2026, 3, 26, 13, 38, 22, 907, DateTimeKind.Unspecified); + string text = localWall.ToString( + culture.DateTimeFormat.ShortDatePattern + " " + longTime.Insert(secondsIndex + 2, ".fff"), + culture); + + Assert.True(MxAccessEventMapper.TryParseSourceTimestamp(text, out DateTime utc)); + Assert.Equal(DateTimeKind.Utc, utc.Kind); + Assert.Equal(DateTime.SpecifyKind(localWall, DateTimeKind.Local).ToUniversalTime(), utc); + } + + /// + /// The parse chain exactly as it stood before PERF-20 added the + /// exact-format stage, used as the parity oracle above. + /// + /// Timestamp string to parse. + /// The parsed UTC timestamp on success. + /// when the string parsed successfully. + private static bool TryParseWithGeneralChainOnly(string? text, out DateTime utc) + { + utc = default; + if (string.IsNullOrWhiteSpace(text)) + { + return false; + } + + const DateTimeStyles styles = DateTimeStyles.AssumeLocal | DateTimeStyles.AdjustToUniversal; + if (DateTime.TryParse(text, CultureInfo.CurrentCulture, styles, out DateTime parsed) + || DateTime.TryParse(text, CultureInfo.InvariantCulture, styles, out parsed)) + { + utc = DateTime.SpecifyKind(parsed, DateTimeKind.Utc); + return true; + } + + return false; + } + private sealed class FakeStatus { public int success; diff --git a/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs b/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs index 036cd9c..9aad71f 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/Conversion/MxStatusProxyConverter.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; +using System.Linq.Expressions; using System.Reflection; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -11,13 +12,15 @@ namespace ZB.MOM.WW.MxGateway.Worker.Conversion; public sealed class MxStatusProxyConverter { /// - /// Per-type cache of the four resolved objects a - /// status conversion needs. The status type is stable (the interop - /// MXSTATUS_PROXY struct in production; a fixed test double in - /// tests), so the expensive - /// metadata scan is resolved once per type and reused. Keyed by - /// so a plain-CLR test double and the real interop - /// struct each get their own entry, keeping the converter interop-agnostic. + /// Per-type cache of the four field accessors a status conversion needs. + /// The status type is stable (the interop MXSTATUS_PROXY struct in + /// production; a fixed test double in tests), so the expensive + /// metadata scan — and, + /// for a visible type, the one-time expression compile that replaces + /// on the event path — happens + /// once per type and is reused. Keyed by so a + /// plain-CLR test double and the real interop struct each get their own + /// entry, keeping the converter interop-agnostic. /// private static readonly ConcurrentDictionary FieldCache = new(); @@ -31,12 +34,11 @@ public sealed class MxStatusProxyConverter throw new ArgumentNullException(nameof(status)); } - Type statusType = status.GetType(); - StatusFields fields = GetFields(statusType); - int success = ReadInt32Field(status, statusType, fields.Success); - int rawCategory = ReadInt32Field(status, statusType, fields.Category); - int rawDetectedBy = ReadInt32Field(status, statusType, fields.DetectedBy); - int detail = ReadInt32Field(status, statusType, fields.Detail); + StatusFields fields = GetFields(status.GetType()); + int success = fields.Success(status); + int rawCategory = fields.Category(status); + int rawDetectedBy = fields.DetectedBy(status); + int detail = fields.Detail(status); return new MxStatusProxy { @@ -109,26 +111,102 @@ public sealed class MxStatusProxyConverter } /// - /// Resolves (and caches) the four objects for the - /// given status type. The first resolution for a type performs the - /// reflection scan; every subsequent conversion of that type reuses the - /// cached entry. A type missing a required field throws the same - /// the per-field lookup used to - /// throw — and, because + /// Resolves (and caches) the four field accessors for the given status + /// type. The first resolution for a type performs the reflection scan and + /// the expression compile; every subsequent conversion of that type + /// reuses the cached delegates. A type missing a required field throws the + /// same the per-field lookup used + /// to throw — and, because /// does not store a value when the factory throws, a bad type keeps /// failing identically on every call rather than being cached. /// /// Runtime type of the status object being converted. - /// The resolved field set for . + /// The resolved field accessors for . private static StatusFields GetFields(Type statusType) { return FieldCache.GetOrAdd( statusType, type => new StatusFields( - ResolveField(type, "success"), - ResolveField(type, "category"), - ResolveField(type, "detectedBy"), - ResolveField(type, "detail"))); + BuildAccessor(type, "success"), + BuildAccessor(type, "category"), + BuildAccessor(type, "detectedBy"), + BuildAccessor(type, "detail"))); + } + + /// + /// Builds the accessor for one status field. Every OnDataChange carries a + /// status array, so this read is on the hot event path: a public field on + /// a publicly visible type is read through a delegate compiled once from + /// an expression tree, which drops the boxed that + /// allocates per field per status + /// entry, plus the dispatch behind + /// . + /// + /// The value is required to be identical, so the compiled path is + /// taken only for field types whose conversion to + /// is a lossless widening (see ) — + /// the interop MXSTATUS_PROXY declares its four fields as + /// 16/32-bit integers. Field types where the CLR conversion and + /// Convert.ToInt32 can disagree (bool, char, floating point, + /// unsigned 32/64-bit, or a reference type that could be null) keep + /// the reflection read verbatim, as does a non-visible type, whose + /// members a compiled delegate is not permitted to touch. + /// + /// + /// Runtime type of the status object. + /// Name of the status field to read. + /// A delegate reading the field as an . + private static Func BuildAccessor( + Type valueType, + string fieldName) + { + FieldInfo field = ResolveField(valueType, fieldName); + if (valueType.IsVisible && IsWideningToInt32(field.FieldType)) + { + try + { + ParameterExpression parameter = Expression.Parameter(typeof(object), "status"); + + // Unbox (not Convert) for a struct: it addresses the boxed + // instance in place rather than copying it out before the load. + Expression instance = valueType.IsValueType + ? Expression.Unbox(parameter, valueType) + : Expression.Convert(parameter, valueType); + Expression widened = Expression.Convert(Expression.Field(instance, field), typeof(int)); + return Expression.Lambda>(widened, parameter).Compile(); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Compiling the accessor is an optimization, never a requirement: + // if the runtime refuses it the reflection read below still + // produces the same value. + } + } + + return status => ReadInt32Field(status, valueType, field); + } + + /// + /// Reports whether reading a field of this type as an + /// via the CLR's own conversion is lossless and total, and therefore + /// indistinguishable from . + /// Enums answer with their underlying type code, which is the intent. + /// + /// Declared type of the status field. + /// when the compiled accessor is safe to use. + private static bool IsWideningToInt32(Type fieldType) + { + switch (Type.GetTypeCode(fieldType)) + { + case TypeCode.SByte: + case TypeCode.Byte: + case TypeCode.Int16: + case TypeCode.UInt16: + case TypeCode.Int32: + return true; + default: + return false; + } } private static FieldInfo ResolveField( @@ -179,17 +257,17 @@ public sealed class MxStatusProxyConverter } /// - /// The four resolved status fields cached per type. Plain readonly struct - /// (not a record) so it compiles under the worker's net48 target, which - /// lacks IsExternalInit. + /// The four resolved status-field accessors cached per type. Plain + /// readonly struct (not a record) so it compiles under the worker's net48 + /// target, which lacks IsExternalInit. /// private readonly struct StatusFields { public StatusFields( - FieldInfo success, - FieldInfo category, - FieldInfo detectedBy, - FieldInfo detail) + Func success, + Func category, + Func detectedBy, + Func detail) { Success = success; Category = category; @@ -197,12 +275,12 @@ public sealed class MxStatusProxyConverter Detail = detail; } - public FieldInfo Success { get; } + public Func Success { get; } - public FieldInfo Category { get; } + public Func Category { get; } - public FieldInfo DetectedBy { get; } + public Func DetectedBy { get; } - public FieldInfo Detail { get; } + public Func Detail { get; } } } diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs index c16b984..d1958cf 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessEventMapper.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Globalization; using Google.Protobuf.WellKnownTypes; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -9,6 +10,18 @@ namespace ZB.MOM.WW.MxGateway.Worker.MxAccess; /// Maps MXAccess COM events to protobuf MxEvent messages. public sealed class MxAccessEventMapper { + /// + /// Per-culture cache of the explicit timestamp formats + /// tries before the general parse + /// chain. Keyed by because the format set is + /// derived from the culture's own patterns and the worker host's culture + /// is fixed in practice, so the derivation runs once. + /// + private static readonly ConcurrentDictionary TimestampFormatCache = new(); + + /// Cached factory delegate so the dictionary lookup allocates nothing on the event path. + private static readonly Func TimestampFormatFactory = BuildTimestampFormats; + private readonly VariantConverter variantConverter; private readonly MxStatusProxyConverter statusProxyConverter; @@ -353,6 +366,23 @@ public sealed class MxAccessEventMapper /// wall-clock UTC). The parsed value is therefore taken as local time /// and converted to UTC. Tries the worker host's culture first /// (MXAccess formats with the host locale), then the invariant culture. + /// + /// PERF: every OnDataChange carries one of these strings, and the + /// general + /// has to walk the culture's whole pattern set to recognize it. The + /// shapes MXAccess actually emits are tried first with + /// . + /// The formats are derived from the current culture's own + /// short-date and long-time patterns (see + /// ) rather than hard-coded to the + /// US shapes seen in captures: a hard-coded M/d/yyyy would + /// reorder month and day on a day-first host relative to what the + /// general parse below returns. Deriving them means any string the + /// exact parse accepts is one the culture-aware general parse would + /// have accepted with the same value, so the fast path is a shortcut + /// and never a different answer; anything it does not match falls + /// through to the unchanged two-stage chain. + /// /// /// The MXAccess timestamp string. /// The parsed UTC timestamp on success. @@ -366,7 +396,10 @@ public sealed class MxAccessEventMapper } const DateTimeStyles styles = DateTimeStyles.AssumeLocal | DateTimeStyles.AdjustToUniversal; - if (DateTime.TryParse(text, CultureInfo.CurrentCulture, styles, out DateTime parsed) + CultureInfo culture = CultureInfo.CurrentCulture; + string[] formats = TimestampFormatCache.GetOrAdd(culture, TimestampFormatFactory); + if (DateTime.TryParseExact(text, formats, culture, styles, out DateTime parsed) + || DateTime.TryParse(text, culture, styles, out parsed) || DateTime.TryParse(text, CultureInfo.InvariantCulture, styles, out parsed)) { utc = DateTime.SpecifyKind(parsed, DateTimeKind.Utc); @@ -376,6 +409,37 @@ public sealed class MxAccessEventMapper return false; } + /// + /// Builds the explicit timestamp formats for a culture: the culture's + /// short date plus its long time, and — first, because it is the shape + /// MXAccess actually fires — the same pattern with the millisecond + /// fraction MXAccess appends to the seconds ("3/26/2026 1:38:22.907 PM" + /// under en-US). No culture publishes a long-time pattern with + /// milliseconds, so the fractional sibling is derived by inserting + /// .fff after the seconds specifier; a pattern without a + /// two-digit seconds specifier, or one that already carries a fraction, + /// gets only the published pattern. + /// + /// Culture whose patterns the formats are derived from. + /// The formats to try, most likely first. + private static string[] BuildTimestampFormats(CultureInfo culture) + { + DateTimeFormatInfo formatInfo = culture.DateTimeFormat; + string shortDate = formatInfo.ShortDatePattern; + string longTime = formatInfo.LongTimePattern; + int secondsIndex = longTime.IndexOf("ss", StringComparison.Ordinal); + if (secondsIndex >= 0 && longTime.IndexOf('f') < 0 && longTime.IndexOf('F') < 0) + { + return + [ + shortDate + " " + longTime.Insert(secondsIndex + 2, ".fff"), + shortDate + " " + longTime, + ]; + } + + return [shortDate + " " + longTime]; + } + private MxArray ConvertBufferedArray( object? value, MxDataType expectedElementDataType)