Repository navigation
perf: typed field accessors, cached TypeCode, and range coverage - #9
Merged
Merged
Conversation
…e orders Each of the five ConverterRangeViolationBehaviour values had exactly one test on one int field. A per type mistake, for example SetToMinValue being right for int and wrong for ushort, would not have been noticed. AllNumericRangesPoco carries sbyte, short, ushort, int, uint, long, ulong, float, double and decimal, each with its own limits and default, in a little endian and a big endian variant. Every behaviour is asserted per field by name, and big endian must produce the same replacement values as little endian. Also pins that None behaves like IgnoreAndContinue rather than zeroing the field, and that a value inside its range never invokes the handler at all. These are read direction paths, so the byte exact golden vectors cannot cover them. This is the safety net for the typed accessor rewrite, where the range check moves onto the accessor and becomes per type code. 72 to 83 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Type.GetTypeCode(kvp.Value.FieldInfo.FieldType) ran for every field of every message in both directions. ConverterFieldInfo already had a private _fieldTypeCode, but it was only populated for range checked fields, so the field existed, was half used, and the hot path recomputed the value anyway. It is public and always resolved now, and both call sites use it. The only remaining Type.GetTypeCode calls happen while preparing the converter. A/B of the same tree with and without the change, back to back: Read LE 125.08 -> 116.38 ns -7.0% Read BE 152.64 -> 143.47 ns -6.0% Read reused 120.23 -> 113.35 ns -5.7% Write LE 240.06 -> 210.17 ns -12.5% Write BE 354.62 -> 341.77 ns -3.6% Read string 75.27 -> 71.53 ns -5.0% Write string 183.39 -> 175.87 ns -4.1% Every row improves, which is what one expects here: both SetFieldValue and WriteFieldValueToArray called GetTypeCode, so there is no row that should have stayed flat. Allocation is unchanged, this removes work and not garbage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The read path assigned through Action<T, object> and the write path read through Func<T, object>, so every value type was boxed once per field per message. The range check went through IsInRange(object) and boxed again. FastInvoke gained BuildTypedFieldSetter and BuildTypedFieldGetter. The existing typed helpers only worked on PropertyInfo via Delegate.CreateDelegate, which is useless for fields because a field has no get or set method. The new ones compile an expression that touches the field directly and fold an optional conversion in. ConverterFieldInfo builds a typed pair per field and exposes Set<TField> and Get<TField>. Casting the stored Delegate to Action<T, int> is a type check, not an allocation. An enum is accessed as int and the compiled expression converts, so Enum.ToObject and the box it returned are gone as well. Converted: 54 setter call sites and 10 range checks on the read side, the whole switch on the write side. IsInRange(object) is no longer called anywhere. The two remaining Setter calls assign strings, which are reference types and never boxed. Read LE 118.86 -> 94.78 ns -20.3% 344 -> 104 B Read BE 148.85 -> 115.31 ns -22.5% 568 -> 328 B Read reused 114.23 -> 90.71 ns -20.6% 288 -> 48 B Write LE 209.75 -> 204.25 ns -2.6% 888 -> 648 B Write BE 333.25 -> 336.31 ns +0.9% 1528 -> 1288 B Read string 71.39 -> 73.42 ns +2.8% 288 -> 264 B Write string 182.10 -> 155.78 ns -14.5% 472 -> 448 B The write path hardly moves in time because it is no longer dominated by the box. What remains is BitConverter.GetBytes allocating a byte[] per field, LINQ Reverse for big endian and the growing List<byte>, which is what Phase 3 removes. Enum accessors are typed on int on purpose. Typing them on the primitive that DestinationType names would not match the read path, which assembles the raw value as an int for every DestinationType it supports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 complete. Three commits, each measured on its own.
1. Range violation coverage first (prerequisite)
Each of the five
ConverterRangeViolationBehaviourvalues had exactly one test on oneintfield. Since the range check was about to move onto per-type accessors, a mistake affectingushortbut notintwould have gone unnoticed — and these are read-direction paths, so the golden vectors are structurally unable to cover them.AllNumericRangesPoconow carries all ten range-capable types (sbyte → decimal) in little- and big-endian variants, every behaviour asserted per field by name. Also pins thatNonebehaves likeIgnoreAndContinuerather than zeroing a field, and that an in-range value never invokes the handler. 72 → 83 tests.2. Cached
TypeCodeType.GetTypeCode(...)ran per field per message in both directions.ConverterFieldInfoalready had a private_fieldTypeCodebut only populated it for range-checked fields — half-used, and the hot path recomputed it anyway.Read −5.7% to −7.0%, Write LE −12.5%, Write BE −3.6%, strings −4% to −5%. Every row improves, which is expected here: both
SetFieldValueandWriteFieldValueToArraycalled it, so no row should have stayed flat.3. Typed accessors — no boxing in either direction
Action<T, object>on read andFunc<T, object>on write boxed every value type once per field per message, andIsInRange(object)boxed again.FastInvokegainedBuildTypedFieldSetter/BuildTypedFieldGetter. The pre-existing typed helpers only worked onPropertyInfoviaDelegate.CreateDelegate, which cannot work for fields — a field has no get/set method. The new ones compile an expression that touches the field directly.Reading into a reused instance allocates 48 bytes instead of 288.
The write path barely moves in time because it is no longer dominated by the box. What remains is
BitConverter.GetBytesallocating abyte[]per field, LINQReversefor big-endian, and the growingList<byte>— exactly what Phase 3 removes.Enum.ToObjectis gone too: enums are accessed asintand the compiled expression converts.Cumulative against shipped 3.6.1 (
bcea658)Golden vectors unmoved throughout. 83/83.
🤖 Generated with Claude Code