Repository navigation
AOT (4/7): core serializers and bindings from a built-in table; serialization-identifiers matched by name — canary goes green - #8604
Conversation
a5e7aac to
b6fc02f
Compare
b6fc02f to
f916473
Compare
f916473 to
28befed
Compare
28befed to
7181ae6
Compare
7181ae6 to
9586b91
Compare
413ee45 to
29e8965
Compare
29e8965 to
0d764f4
Compare
Aaronontheweb
left a comment
There was a problem hiding this comment.
Mostly looks ok, with some nitpicks
| // System.Object binding that points at it, and a type with no binding of its own has no fallback. That | ||
| // is the designed behavior, not a gap - so assert the throw, and assert the message tells the user what | ||
| // to do about it. | ||
| var unbound = RequireThrows(label, () => system.Serialization.FindSerializerFor("hello")); |
There was a problem hiding this comment.
LGTM - the comment above is correct about both the design and the assertion.
|
|
||
| /// <summary> | ||
| /// A serializer that needs no reflection, so it is a legitimate thing to register under AOT - which is | ||
| /// exactly the migration the ledger recommends for a switched-off application. |
There was a problem hiding this comment.
LGTM - good thing to test outside all of the source-generated serialization infrastructure in Akka.NET v1.6
|
|
||
| serialization.FindSerializerForType(typeof(byte[])).Should().BeOfType<ByteArraySerializer>(); | ||
|
|
||
| // "System.Object" = json, so an otherwise unbound type falls back to Newtonsoft.Json |
| /// serializer that throws the first time anything is serialized. An AOT application that wants JSON | ||
| /// registers a serializer of its own through a <see cref="SerializationSetup"/>. | ||
| /// </summary> | ||
| private static object CreateNewtonSoftJsonSerializer(ExtendedActorSystem system, Config config) |
There was a problem hiding this comment.
these serializers have distinct base classes we can use - why not use that here instead of returning object ?
There was a problem hiding this comment.
Done in 6cce8a4. The built-in table and both factories now return Serializer, the shared base of V1 and V2 serializers.
| // Whether a built-in binding survives is decided by the alias, not by the feature switch: | ||
| // core's own "System.Object" = json row lands here when json was skipped, which is expected, | ||
| // but the very same row pointed at a SerializationSetup alias must still be honored. | ||
| if (!skippedAliases.Contains(serializerName)) |
| } | ||
|
|
||
| [RequiresUnreferencedCode("Loads a serializer named under [akka.actor.serializers] by name and activates it. The trimmer cannot tell which type that is, so it may have been trimmed away.")] | ||
| private static object CreateSerializerFromTypeName(string serializerTypeName, ExtendedActorSystem system, Config serializerConfig) |
There was a problem hiding this comment.
again, return the concrete base type rather than object - although, how does this handle Serializer V1 vs V2?
There was a problem hiding this comment.
Done in 6cce8a4. CreateSerializerFromTypeName now returns SerializerV2: it passes the Activator result through AdaptSerializer, because a type named in HOCON is not guaranteed to be a serializer.
V1 vs V2: SerializerV2 derives from Serializer, and the constructor stores every serializer as a SerializerV2 via AdaptSerializer:
- a
SerializerV2passes through unchanged (checked first) - a V1
Serializeris wrapped inSerializerV1Adapter - anything else throws the "must inherit from Serializer or SerializerV2"
ArgumentException
ByteArraySerializer is already V2; NewtonSoftJsonSerializer is V1 and gets wrapped. Calling AdaptSerializer twice on the reflection path is a no-op the second time.
Also in this push (7c3c1ed): with dynamic type loading off, a SerializationSetup now covers the HOCON rows it takes over. Before, the HOCON loop threw on a module alias before the Setup loop ran, so the fix the error message suggests could never work.
7c3c1ed to
55dd3e4
Compare
…ving every key SerializerIdentifierHelper.GetSerializerIdentifierFromConfig already holds the Type whose id it wants, so it now formats that Type and compares it against each configured key instead of calling Type.GetType(key, throwOnError: true) on every key in the block. Each key is trimmed, run through TypeExtensions.StripAssemblyIdentity, then split at the comma separating the type name from the assembly name - the first comma at bracket depth zero, so a closed generic's own commas do not count. The type name compares Ordinal, the assembly name OrdinalIgnoreCase, which is how Type.GetType compared assembly names. Assembly-qualified keys are matched across the whole block first and bare FullName keys only afterwards, so a bare key cannot shadow an exact qualified key further down the block. That drops the last Type.GetType call on the serialization startup path and fixes a latent bug along the way. Serialization..ctor reads Serializer.Identifier for every serializer it registers, so one key naming a type the application cannot load - a serializer configured by a package that is not referenced - took ActorSystem.Create down with it, however well-formed the other keys were. The spec for it fails on dev inside ActorSystem.Create, not at the assertion. StripAssemblyIdentity is the shared helper introduced with the feature switch, and TypeQualifiedName builds on it too, so its output is a wire manifest. It was therefore measured rather than eyeballed before this site started depending on it: old stripping against new over 5,544 real AssemblyQualifiedNames - every type in Akka.dll, System.Private.CoreLib, System.Linq and Akka.Tests, plus hand-picked closed generics, jagged arrays and nested generics - is byte-identical in every case. The narrowing and the two widenings are recorded in BREAKING_CHANGES_V1.6.md.
Akka.NET's own akka.conf declares two serializers under akka.actor.serializers - bytes and json - and two bindings under akka.actor.serialization-bindings - System.Byte[] and System.Object. Both HOCON loops resolved those names with Type.GetType, so the trimmer could not tell which types were being loaded and dropped them; the unrooted AOT canary logged four "did not resolve" warnings at startup and booted with no serializers and two dangling bindings. Both loops now consult a BuiltIn* table first and construct the type directly. A name that misses the table falls back to the old reflection code, moved into a private static method marked [RequiresUnreferencedCode] and reachable only while dynamic type loading is on; with the switch off the loop throws a ConfigurationException built by AkkaFeatures.NotBuiltIn. BuiltInSerializers carries the bare and the assembly-qualified spelling of each name; BuiltInSerializationBindings carries the bare spelling plus mscorlib, System.Private.CoreLib, System.Runtime and netstandard, because these two are framework types with no single qualified spelling and Type.GetType resolved all four. Versioned spellings need no keys of their own: the lookup runs the configured name through StripAssemblyIdentity first. That is deliberately in preference to a third key built from typeof(T).AssemblyQualifiedName - measured on the canary, the typeof keys cost 16 bytes (5,370,888 vs 5,370,872) and neither variant produced a NewtonSoftJsonSerializer.cs trim warning, so the reason is not size but that stripping matches every version of a name where a typeof-built key only matches the build it came from. With the switch on nothing changes: json and the System.Object binding are registered exactly as today. With it off, core does not register the json alias at all - Newtonsoft.Json is reflection-driven from top to bottom, and leaving it out beats registering a serializer that throws the first time anything is serialized. A type with no binding of its own then has no fallback serializer and FindSerializerForType throws through the path allow-unregistered-types already had, with an added sentence naming the switch and pointing at SerializationSetup, because "Serializer not found for type Poco" on a default config is not something a user can act on. Crucially, whether a built-in binding survives is decided by the alias it points at, not by the feature switch. Skipping the System.Object binding from the switch silently broke the migration the ledger recommends - a SerializationSetup alias plus "System.Object" = mine, where mine IS registered, since setups are added before the bindings loop. The serializers loop now records any alias whose built-in factory declined in a local skippedAliases set, and a binding whose target is missing warns as before unless the target is in that set. So "System.Object" = json is dropped silently with the switch off, "System.Object" = mine is honored, and "System.Object" = typo still warns. ByteArraySerializer has only a (ExtendedActorSystem) constructor. On dev an akka.actor.serialization-settings.bytes block pushed Activator.CreateInstance down the two-argument branch and threw MissingMethodException; the table constructs it directly and ignores the block, which is better and is ledgered. Serialization-bindings keys are trimmed - HOCON trims values but not keys, and a binding's type name is the key - which is a widening, since Type.GetType rejected a padded bare name. Two keys landing on the same Type stay harmless: AddSerializationMap is last-write-wins, as it already was for the several spellings that resolved to the same Type on their own. Recorded in BREAKING_CHANGES_V1.6.md. The AOT canary asserted that a string resolved to a serializer, which was true only while json was registered unconditionally. That assertion now pins the ruling instead of contradicting it: byte[] must still resolve, and serializing an unbound type must throw a SerializationException whose message names the feature switch and SerializationSetup. README pass condition updated to match.
… row and reuse the shared switch name Serialization.cs: HOCON trims values but not keys. Trimming a serialization-bindings key accepted padded keys that dev rejected, a widening that has nothing to do with the built-in tables and that the mailbox requirement keys deliberately do not make. Use kvp.Key as written, as before. Serializer.cs GetSerializerIdentifierFromConfig: same for serialization-identifiers keys - drop the .Trim() before StripAssemblyIdentity in both passes. The .TrimEnd()/.Trim() on the two halves after the comma split stay; they are what turn "Ns.T, Akka" into "Ns.T" and "Akka". BREAKING_CHANGES_V1.6.md: drop the clause saying a padded serialization-bindings key now resolves. BuiltInSerializerDefaultsSpec.cs: use AkkaFeaturesSpec.SwitchName instead of a local copy.
… object The built-in factories and the reflection fallback now return the serializer base type. CreateSerializerFromTypeName adapts its result, so a type named in HOCON that is not a serializer still fails with the AdaptSerializer message, and a V1 serializer is still wrapped.
… type loading is off With Akka.DynamicTypeLoading off, the serializer loop threw on any HOCON alias core could not resolve before the SerializationSetup loop ran, so the fix the error message suggests could never work for a row that a module's reference.conf still carries. - A serializer row is skipped when a SerializationSetup registers the same alias; the Setup registers it right after. - A binding row resolves without reflection when its type name matches one of the Setup's UseFor types (bare full name, name plus assembly, or a full assembly-qualified name). - Any other row still throws. The switch-on path is unchanged.
55dd3e4 to
a1574c6
Compare
Changes
Part of the milestone-1 AOT stack. Stacked on
aot/m1-a-feature-switch-and-first-tables(plus B and C).Two commits, both confined to the serialization subsystem.
1. Match
serialization-identifiersentries by type name instead of resolving every keySerializerIdentifierHelper.GetSerializerIdentifierFromConfigalready holds theTypewhose id it wants. Itused to build a dictionary by calling
Type.GetType(key, throwOnError: true)on every key underakka.actor.serialization-identifiers, then look the type up in it. Now it formats the type it has and comparesthat against each key.
That drops the last
Type.GetTypeon the serialization startup path, and it fixes a latent bug on the way:because
Serialization..ctorreadsSerializer.Identifierfor every serializer it registers, one key naminga type the application cannot load — a serializer configured by a package that is present in HOCON but not
referenced — made
ActorSystem.Createitself throw, no matter how well-formed every other key was. The spec forthis fails on
devatActorSystem.Create, not at the assertion.How a key is matched
Each key is trimmed, run through the new
TypeExtensions.StripAssemblyIdentity(stripsVersion,Culture,PublicKeyToken,ProcessorArchitecture,Retargetable,ContentType), then split at the comma that separatesthe type name from the assembly name — the first comma at bracket depth zero, so a closed generic's own commas
don't count. The type name is compared
Ordinal; the assembly nameOrdinalIgnoreCase, which is howType.GetTypecompared assembly names.Assembly-qualified keys are matched across the whole block first, and only then bare
FullNamekeys, so abare key cannot shadow an exact assembly-qualified key further down the block. There is a spec for that ordering.
What still matches that matched on
dev"Ns.T"(bare),"Ns.T, Asm", and"Ns.T, Asm, Version=…, Culture=…, PublicKeyToken=…""Ns.T,Asm"— no space after the comma"Ns.T, asm"— assembly name in the wrong case"Ns.T, Asm, …, ProcessorArchitecture=MSIL, Retargetable=Yes""Ns.Outer+Nested, Asm"— nested types spell with+on both sidesFullNameis stripped tooAll of these are in the
[Theory].What no longer matches (the narrowing)
String comparison cannot follow assembly binding, so a key that named the assembly some other way that
Type.GetTypenevertheless resolved now misses and that serializer's id lookup throwsArgumentException:"Akka.Serialization.X, SomeOldAssembly"whereSomeOldAssemblyforwards the typeto
Akka[[…]]spelled with a partial or different assembly nameWhat newly matches (the widenings)
dev, a key that named a strong-named assembly with the wrongVersion/PublicKeyTokenfailed to bind and (given thethrowOnError: true) blew up the whole block. Now itmatches. This is the intended behavior —
TypeQualifiedName(), which is what Akka.NET itself writes as a wiremanifest, strips exactly these components.
FullNamekey matches a type of that name in any assembly. Ondev,Type.GetTypesearched onlyAkka.dlland corlib for an unqualified name, so a bare key naming a type in a third assembly threw. The riskthis creates is worth stating plainly: two serializers both called
Foo.MySerializer, in different assemblies,would now both match a single bare
"Foo.MySerializer"key and share its id — and a shared serializer idcorrupts the wire. Use assembly-qualified keys.
Shared identity stripping
The lookup uses
Akka.Util.TypeExtensions.StripAssemblyIdentity, the[GeneratedRegex]helper PR #8601 (A)introduced for every built-in table;
TypeQualifiedNamecalls the same helper, so wire manifests andconfig matching agree on what "assembly identity" means. A's commit message carries the 5,544-name
byte-identical measurement that validates the rewrite against the old regex.
2. Register core's built-in serializers and bindings without reflection
Core's own
akka.confdeclares two serializers (bytes,json) and two bindings (System.Byte[],System.Object). Both HOCON loops resolved those names throughType.GetType, so the trimmer could not tellwhich types were loaded and dropped them: the unrooted AOT canary logged four "did not resolve" warnings at
startup and booted with no serializers and two dangling bindings.
Both loops now consult a
BuiltIn*table first and construct the type directly, per the milestone's resolutionorder: table →
else if (AkkaFeatures.IsDynamicTypeLoadingSupported)the old reflection code, moved into aprivate staticmethod marked[RequiresUnreferencedCode]→else throwaConfigurationExceptionbuilt byAkkaFeatures.NotBuiltIn, naming the setting, the value and the switch.Table keys
BuiltInSerializerscarries the bare and the assembly-qualified spelling of each name.BuiltInSerializationBindingscarries the bare spelling plus
mscorlib,System.Private.CoreLib,System.Runtimeandnetstandard— thesetwo are framework types, so there is no single assembly-qualified spelling, and all four resolve through
Type.GetType. Versioned spellings need no keys of their own because the lookup runs the configured name throughStripAssemblyIdentityfirst.That is deliberately in preference to a third key built from
typeof(T).AssemblyQualifiedName. Both werepublished on the canary: with the
typeofkeys the binary was 5,370,888 bytes, without them 5,370,872, andneither produced any
NewtonSoftJsonSerializer.cstrim warning. So the reason is not AOT size — it is thatstripping matches every version of a name, where a
typeof-built key only ever matches the build it came from.Newtonsoft under the switch
With the switch on nothing changes:
jsonand theSystem.Objectbinding are registered exactly as today.With the switch off, core does not register
jsonat all.NewtonSoftJsonSerializeris reflection-driven fromtop to bottom, and leaving it out beats registering a serializer that throws the first time anything is
serialized. A type with no binding of its own then has no fallback serializer, and
FindSerializerFor/FindSerializerForTypethrow through the pathallow-unregistered-typesalready had — with an added sentencesaying why, because
SerializationException: Serializer not found for type Pocoon a default config is notsomething a user can act on:
The
System.Objectbinding is decided by the alias, not the switchThis is the fix for the one blocker the review caught. An earlier revision skipped the
System.Objectbindingwhenever the switch was off — which silently broke the very migration the ledger recommends: a
SerializationSetupalias plus
"System.Object" = minein HOCON, wheremineis registered (setups are added before the bindingsloop). The row was dropped before
_serializersByNamewas ever consulted.Now the bindings table is unconditional, and the serializers loop records in a local
skippedAliasesset anyalias whose built-in factory declined to produce a serializer. A binding whose target is missing warns as before
unless the target is in that set. So:
"System.Object" = json, switch off → dropped, no warning (json was deliberately skipped)"System.Object" = minewithminefrom aSerializationSetup, switch off → honored"System.Object" = typo→ still warnsSerialization binding to non existing serializer: 'typo'Both the honored case and the silence are specs, and both fail against the previous revision.
Other behavior notes
akka.actor.serialization-settingsblock surfaces its own exception instead of theTargetInvocationExceptionActivator.CreateInstancewrapped it in.ByteArraySerializerhas only a(ExtendedActorSystem)constructor. Ondev, anakka.actor.serialization-settings.bytesblock pushed theActivatorcall down the two-argument branch andthrew
MissingMethodException; the table now constructs it directly and ignores the block. Kept, becausebooting beats crashing on a setting that never did anything, and ledgered.
serialization-bindingskeys are trimmed. HOCON trims values but not keys, and a binding's type name is thekey, so this is a widening:
Type.GetTyperejected a padded bare name, making a padded key warn-and-skipbefore. Two keys landing on the same
Typeis harmless —AddSerializationMapis last-write-wins, which italready was for the several distinct spellings that resolved to the same
Typeon their own..Trim()on config values: HOCON'sGetStringalready trims those in every syntax, so a trim therewould be dead code.
Verification
dotnet build src/core/Akka/Akka.csproj -c Release -warnaserror→ 0/0;dotnet build Akka.slnx -c Release→ 0/0Akka.Tests,Akka.Remote.Tests --filter ~Serialization,Akka.Persistence.Tests --filter ~SerializAkka.API.Testspasses unchanged — no public API changedotnet format --verify-no-changeson every touched file: no new violationsSerialization.cs/Serializer.csnow produce zeroIL2xxx/IL3xxxwarnings (were
Serialization.cs:232,Serialization.cs:259,Serializer.cs:232). The four serializer/binding"did not resolve" startup warnings are gone.
[canary] OKand exits 0 with zero startup warnings (assertedby the watchdog).
AssertBuiltInsResolvednow pins the switch-off contract:byte[]resolves; an unboundtype (
string) throws aSerializationExceptionnamingAkka.DynamicTypeLoadingandSerializationSetup.Akka-own IL warnings on the canary publish: 21 → 9, all in PR E's sites or later.
Stack: PR 4 of 7 for AOT milestone 1. Base is PR #8603 (C); this PR shows only its own two commits. This is the PR at which the milestone-1 canary goes green. Design and measurements: epic #7246.
Checklist