diff --git a/holonsoft.FastProtocolConverter.Performance/Benchmarks.cs b/holonsoft.FastProtocolConverter.Performance/Benchmarks.cs index fd6478c..ac128ff 100644 --- a/holonsoft.FastProtocolConverter.Performance/Benchmarks.cs +++ b/holonsoft.FastProtocolConverter.Performance/Benchmarks.cs @@ -35,6 +35,13 @@ public class ConverterBenchmarks /// frame out of here into its own byte[] before the converter would look at it. /// private byte[] _receiveBuffer; + + private IProtocolConverter _advanced; + private IProtocolConverter _advancedBigEndian; + private BenchmarkAdvancedPoco _advancedSource; + private BenchmarkAdvancedPocoBigEndian _advancedBigEndianSource; + private byte[] _advancedPayload; + private byte[] _advancedBigEndianPayload; private int _frameOffset; private int _frameLength; @@ -96,6 +103,34 @@ public void Setup() _frameLength = _littlePayload.Length; _receiveBuffer = new byte[_frameOffset + _frameLength + 5]; _littlePayload.CopyTo(_receiveBuffer, _frameOffset); + + _advanced = new ProtocolConverter(null); + _advanced.Prepare(); + + _advancedBigEndian = new ProtocolConverter(null); + _advancedBigEndian.Prepare(); + + var stamp = new DateTime(2026, 9, 13, 21, 47, 11, DateTimeKind.Utc); + var guid = new Guid("0f8fad5b-d9cb-469f-a165-70867728950e"); + + _advancedSource = new BenchmarkAdvancedPoco + { + DateTime32Field = stamp, + DateTime64Field = stamp, + GuidField = guid, + DecimalField = -12345.6789m, + }; + + _advancedBigEndianSource = new BenchmarkAdvancedPocoBigEndian + { + DateTime32Field = stamp, + DateTime64Field = stamp, + GuidField = guid, + DecimalField = -12345.6789m, + }; + + _advancedPayload = _advanced.ConvertToByteArray(_advancedSource); + _advancedBigEndianPayload = _advancedBigEndian.ConvertToByteArray(_advancedBigEndianSource); } @@ -146,6 +181,26 @@ public byte[] WriteBigEndian() => _bigEndian.ConvertToByteArray(_bigSource); + [Benchmark(Description = "Read DateTime/Guid/decimal POCO, little endian")] + public BenchmarkAdvancedPoco ReadAdvanced() + => _advanced.ConvertFromByteArray(_advancedPayload); + + + [Benchmark(Description = "Read DateTime/Guid/decimal POCO, big endian")] + public BenchmarkAdvancedPocoBigEndian ReadAdvancedBigEndian() + => _advancedBigEndian.ConvertFromByteArray(_advancedBigEndianPayload); + + + [Benchmark(Description = "Write DateTime/Guid/decimal POCO, little endian")] + public byte[] WriteAdvanced() + => _advanced.ConvertToByteArray(_advancedSource); + + + [Benchmark(Description = "Write DateTime/Guid/decimal POCO, big endian")] + public byte[] WriteAdvancedBigEndian() + => _advancedBigEndian.ConvertToByteArray(_advancedBigEndianSource); + + [Benchmark(Description = "Read POCO with fixed length string")] public BenchmarkStringPoco ReadWithString() => _withStrings.ConvertFromByteArray(_stringPayload); @@ -157,6 +212,42 @@ public byte[] WriteWithString() } + /// + /// The types that have their own handler and that no other benchmark POCO covers: DateTime in + /// both unix formats, Guid and decimal. Without this the DateTime path, which reaches into + /// holonsoft.FluentDateTime, and the Guid and decimal paths were never measured at all. + /// + public class BenchmarkAdvancedPoco + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp32Bit)] + public DateTime DateTime32Field; + + [ProtocolField(StartPos = 4)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp64Bit)] + public DateTime DateTime64Field; + + [ProtocolField(StartPos = 12)] public Guid GuidField; + [ProtocolField(StartPos = 28)] public decimal DecimalField; + } + + + [ProtocolSetupArgument(UseBigEndian = true)] + public class BenchmarkAdvancedPocoBigEndian + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp32Bit)] + public DateTime DateTime32Field; + + [ProtocolField(StartPos = 4)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp64Bit)] + public DateTime DateTime64Field; + + [ProtocolField(StartPos = 12)] public Guid GuidField; + [ProtocolField(StartPos = 28)] public decimal DecimalField; + } + + public enum BenchmarkEnum { First, diff --git a/holonsoft.FastProtocolConverter.Performance/baseline-before-v4.md b/holonsoft.FastProtocolConverter.Performance/baseline-before-v4.md index 1600aaa..1710e0f 100644 --- a/holonsoft.FastProtocolConverter.Performance/baseline-before-v4.md +++ b/holonsoft.FastProtocolConverter.Performance/baseline-before-v4.md @@ -271,6 +271,52 @@ The reads are now faster than the write path for the first time in this library' Reading a frame out of a receive buffer without copying it first is 132.05 ns -> 72.42 ns against what a caller had to write before, and 60.55 ns into a reused instance. +## Phase 4, step 3: the DateTime path + +The same non optimized package problem applied to `holonsoft.FluentDateTime`, which sat in both +conversion directions and which **no benchmark covered**: `BenchmarkPoco` has no DateTime field, so +nothing in the suite ever ran that code. Guid and decimal were unmeasured for the same reason. +A `BenchmarkAdvancedPoco` with two timestamps, a Guid and a decimal was added first, so the change +could be measured instead of assumed. + +The two uses turned out to be very different: + +* reading used `DateTimeExtensions.UnixEpoch`, a `static readonly DateTime`. Reading a static field + costs nothing even in an unoptimized assembly, there is no call to inline. +* writing used `dtf.ToUnixTimeSeconds()`, a real extension method, so a non inlinable call into the + unoptimized assembly for every DateTime field of every message. + +Both were replaced by their in box equivalents, `DateTime.UnixEpoch` and +`new DateTimeOffset(dtf).ToUnixTimeSeconds()`, which is literally what the extension method did. +The package reference is gone, so `holonsoft.FluentDateTime` no longer reaches a consumer of this +library at all. + +`DateTime.UnixEpoch` is `1970-01-01T00:00:00` with `DateTimeKind.Utc`, byte for byte and kind for +kind the same value FluentDateTime declared, so nothing about the decoded value changes. + +A/B of the same tree with and without the change, back to back: + +| Method | FluentDateTime | in box | delta | +|---|---:|---:|---:| +| Write DateTime/Guid/decimal LE | 66.70 ns | 60.04 ns | **-10.0%** | +| Write DateTime/Guid/decimal BE | 65.51 ns | 61.27 ns | -6.5% | +| Read DateTime/Guid/decimal LE | 58.18 ns | 59.42 ns | +2.1% | +| Read DateTime/Guid/decimal BE | 58.66 ns | 58.08 ns | -1.0% | + +The other **ten rows are the control**: the change cannot reach them and they all stay within 3%, +drifting slightly upward in the second run. The two write rows moved outside their error bars and +against that drift, the two read rows did not move, which is what the code predicts. About 3 ns +per DateTime field written. + +### Why the golden vectors cannot cover this + +The `Kind` of a decoded DateTime is not in the bytes. A decoder that returns the right instant with +the wrong `Kind` writes the very same frame back out, so a byte exact vector stays green while +every consumer that hands the value to a `DateTimeOffset` silently reinterprets it as local time. +`ProtocolDateTimeField` declares the Kind, which makes it part of the protocol contract, so +`TestDateTimeConversion` asserts it directly, along with round trips in both widths, both byte +orders, and a timestamp before the epoch. + ## A note for the other holonsoft packages `holonsoft.FluentConditions` 3.0.1, `holonsoft.FluentDateTime` 2.1.1 and `holonsoft.Utils` 1.10.1 diff --git a/holonsoft.FastProtocolConverter.Test/TestDateTimeConversion.cs b/holonsoft.FastProtocolConverter.Test/TestDateTimeConversion.cs new file mode 100644 index 0000000..c04300d --- /dev/null +++ b/holonsoft.FastProtocolConverter.Test/TestDateTimeConversion.cs @@ -0,0 +1,190 @@ +using holonsoft.FastProtocolConverter.Abstractions.Attributes; +using holonsoft.FastProtocolConverter.Abstractions.Enums; +using holonsoft.FastProtocolConverter.Abstractions.Interfaces; +using holonsoft.FastProtocolConverter.Test.dto; +using Microsoft.Extensions.Logging; +using Shouldly; +using System; +using Xunit; + +namespace holonsoft.FastProtocolConverter.Test +{ + /// + /// The DateTime handlers, in both unix formats and both kinds. + /// + /// The golden vectors cannot cover this. They pin the bytes, and the Kind of a decoded DateTime + /// is not in the bytes: a decoder that returns the right instant with the wrong Kind writes the + /// very same frame back out. That matters because declares + /// the Kind, so it is part of the protocol contract and not an implementation detail. + /// + public class TestDateTimeConversion + { + private readonly ILogger _logger = null; + + + private static IProtocolConverter CreateConverter(ILogger logger) + where TPoco : class, new() + { + var converter = new ProtocolConverter(logger) as IProtocolConverter; + converter.Prepare(); + return converter; + } + + + /// + /// A decoded timestamp is built from the unix epoch, which is UTC, so the result has to be + /// UTC as well. An Unspecified Kind here would be silently reinterpreted as local time by + /// the next DateTimeOffset conversion, which is a real bug in a distributed system. + /// + [Theory] + [InlineData(DateTimeByteFormat.UnixTimeStamp32Bit)] + [InlineData(DateTimeByteFormat.UnixTimeStamp64Bit)] + public void TestDecodedTimestampIsUtc(DateTimeByteFormat format) + { + var converter = format == DateTimeByteFormat.UnixTimeStamp32Bit + ? (object) CreateConverter(_logger) + : CreateConverter(_logger); + + var stamp = new DateTime(2026, 9, 13, 21, 47, 11, DateTimeKind.Utc); + + if (converter is IProtocolConverter c32) + { + var result = c32.ConvertFromByteArray(c32.ConvertToByteArray(new UtcTimestamp32Poco { Stamp = stamp })); + + result.Stamp.Kind.ShouldBe(DateTimeKind.Utc); + result.Stamp.ShouldBe(stamp); + } + else + { + var c64 = (IProtocolConverter) converter; + var result = c64.ConvertFromByteArray(c64.ConvertToByteArray(new UtcTimestamp64Poco { Stamp = stamp })); + + result.Stamp.Kind.ShouldBe(DateTimeKind.Utc); + result.Stamp.ShouldBe(stamp); + } + } + + + /// + /// The exact instant has to survive a round trip to the second, in both widths. + /// + [Fact] + public void TestRoundTripKeepsTheInstant() + { + var converter = CreateConverter(_logger); + + foreach (var stamp in new[] + { + new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc), + new DateTime(2000, 2, 29, 12, 0, 0, DateTimeKind.Utc), + new DateTime(2026, 9, 13, 21, 47, 11, DateTimeKind.Utc), + new DateTime(2100, 12, 31, 23, 59, 59, DateTimeKind.Utc), + }) + { + var result = converter.ConvertFromByteArray( + converter.ConvertToByteArray(new UtcTimestamp64Poco { Stamp = stamp })); + + result.Stamp.ShouldBe(stamp, $"round trip of {stamp:O}"); + } + } + + + /// + /// A timestamp before the epoch is a negative unix value, which the 32 bit format stores as + /// a signed int. It used to be easy to lose the sign on the way through the cast chain. + /// + [Fact] + public void TestTimestampBeforeTheEpochSurvives() + { + var converter = CreateConverter(_logger); + + var stamp = new DateTime(1960, 6, 15, 8, 30, 0, DateTimeKind.Utc); + + var result = converter.ConvertFromByteArray( + converter.ConvertToByteArray(new UtcTimestamp32Poco { Stamp = stamp })); + + result.Stamp.ShouldBe(stamp); + result.Stamp.Kind.ShouldBe(DateTimeKind.Utc); + } + + + /// + /// A field declared as local time is normalised to local before it is written, so the + /// instant, not the wall clock reading, is what travels. Reading gives UTC back, which is + /// the same instant. + /// + [Fact] + public void TestLocalKindWritesTheSameInstant() + { + var utcConverter = CreateConverter(_logger); + var localConverter = CreateConverter(_logger); + + var stamp = new DateTime(2026, 9, 13, 21, 47, 11, DateTimeKind.Utc); + + var utcBytes = utcConverter.ConvertToByteArray(new UtcTimestamp64Poco { Stamp = stamp }); + var localBytes = localConverter.ConvertToByteArray(new LocalTimestamp64Poco { Stamp = stamp }); + + // same moment in time, so the same number of seconds since the epoch + localBytes.ShouldBe(utcBytes); + } + + + /// + /// Big endian has its own read and write branch for both widths. + /// + [Fact] + public void TestBigEndianTimestampRoundTrips() + { + var converter = CreateConverter(_logger); + + var stamp = new DateTime(2026, 9, 13, 21, 47, 11, DateTimeKind.Utc); + + var bytes = converter.ConvertToByteArray(new UtcTimestamp64BigEndianPoco { Stamp = stamp }); + var result = converter.ConvertFromByteArray(bytes); + + result.Stamp.ShouldBe(stamp); + result.Stamp.Kind.ShouldBe(DateTimeKind.Utc); + + // and the byte order really is the other way round + var little = CreateConverter(_logger) + .ConvertToByteArray(new UtcTimestamp64Poco { Stamp = stamp }); + + bytes.ShouldNotBe(little); + Array.Reverse(little); + bytes.ShouldBe(little); + } + } + + + public class UtcTimestamp32Poco + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp32Bit)] + public DateTime Stamp; + } + + + public class UtcTimestamp64Poco + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp64Bit)] + public DateTime Stamp; + } + + + public class LocalTimestamp64Poco + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Local, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp64Bit)] + public DateTime Stamp; + } + + + [ProtocolSetupArgument(UseBigEndian = true)] + public class UtcTimestamp64BigEndianPoco + { + [ProtocolField(StartPos = 0)] + [ProtocolDateTimeField(DateTimeKind = DateTimeKind.Utc, DateTimeByteFormat = DateTimeByteFormat.UnixTimeStamp64Bit)] + public DateTime Stamp; + } +} diff --git a/holonsoft.FastProtocolConverter/ProtocolConverterFromByteArray.cs b/holonsoft.FastProtocolConverter/ProtocolConverterFromByteArray.cs index 0f88cfb..2b85d1a 100644 --- a/holonsoft.FastProtocolConverter/ProtocolConverterFromByteArray.cs +++ b/holonsoft.FastProtocolConverter/ProtocolConverterFromByteArray.cs @@ -8,7 +8,6 @@ using holonsoft.FastProtocolConverter.Abstractions.Exceptions; using holonsoft.FastProtocolConverter.dto; using holonsoft.FluentConditions; -using holonsoft.FluentDateTime.DateTime; using System.Linq; namespace holonsoft.FastProtocolConverter @@ -825,7 +824,7 @@ private int SetFieldHandleDateTimeValues(T result, KeyValuePair result, KeyValuePair(data); dtf = field.DateTimeAttribute.DateTimeKind == DateTimeKind.Utc ? dtf.ToUniversalTime() : dtf.ToLocalTime(); - var uts = dtf.ToUnixTimeSeconds(); + // this is what holonsoft.FluentDateTime's ToUnixTimeSeconds does, inlined here. That + // package is a non optimized build, so the JIT could neither optimize nor inline the + // call, and the Kind is already normalised to Utc or Local on the line above, which is + // what makes the DateTimeOffset conversion well defined + var uts = new DateTimeOffset(dtf).ToUnixTimeSeconds(); if (field.DateTimeAttribute.DateTimeByteFormat == DateTimeByteFormat.UnixTimeStamp32Bit) { diff --git a/holonsoft.FastProtocolConverter/holonsoft.FastProtocolConverter.csproj b/holonsoft.FastProtocolConverter/holonsoft.FastProtocolConverter.csproj index 7200966..46fd38d 100644 --- a/holonsoft.FastProtocolConverter/holonsoft.FastProtocolConverter.csproj +++ b/holonsoft.FastProtocolConverter/holonsoft.FastProtocolConverter.csproj @@ -1,4 +1,4 @@ - + $(LibraryTargetFrameworks) @@ -8,7 +8,6 @@ -