Skip to content

perf(stjm): typed migrator invokers to remove boxing, per-call GetService and the discriminator scan #208

Closed

Description

@egil

The migration path (not the happy path) goes through untyped plumbing that boxes and re-resolves on every call:

  • IMigratorInvoker.TryMigrate(object? source, out object? migrated) boxes struct sources and struct targets, and does a source is TSource type test per call (StaticMigratorInvoker, ExternalMigratorInvoker).
  • JsonMigratableConverter<T>.ReadSource reads the source through StjInternals.ReadAsObject, which boxes struct sources; the typed JsonConverter<TSource>.Read is available once the source type is known statically.
  • ExternalMigratorInvoker.ResolveMigrator calls serviceProvider.GetService(migratorType) on every migration.
  • FindMigratorByDiscriminator is a linear ValueTextEquals scan over context.Migrators.
  • SetMigrationTracking does an is IJsonMigrationTracked interface test per read, including the happy path; a static readonly bool computed from typeof(T) folds away at JIT time.

Change

  • Make the per-source reference generic: MigratorReference<TSource, TTarget> (created once at converter build time) exposing a typed bool TryRead(ref Utf8JsonReader, out TTarget) that calls JsonConverter<TSource>.Read and then TTarget.TryMigrateFrom (static abstract) or the external IMigrate<TSource, TTarget> instance directly. Keep a non-generic base for the discriminator/shape metadata the read path iterates over.
  • Keep DI semantics (scoped migrators are resolved per call by design, see docs/recipes/dependency-injection.md) but avoid the GetService round trip when no service provider is configured, and avoid the Lazy<T> indirection on the fallback path.
  • Replace the linear discriminator scan with a length-bucketed or first-byte-switched lookup when there are more than a handful of migrators; keep the single-migrator path branch-free.
  • Cache the IJsonMigrationTracked check per T.
  • Benchmarks: add a struct source/target scenario and a "many versions" scenario (for example 8 registered sources) so the boxing and scan changes are visible; refresh docs/perf/*.md.
  • Coverage stays at 100% branch for core components per Egil.SystemTextJson.Migration/AGENTS.md.

This work is independent of #207 but touches JsonMigratableConverterFactory.BuildMigratorMap, so land it after or rebase on it.

Activity

  1. added
    enhancementNew feature or request
    stjmEgil.SystemTextJson.Migration scope
    on Sep 21, 2026
  2. egil commented on Sep 24, 2026

    @egil
    OwnerAuthor

    Resolved by #223, merged as 9be9a3581f4c3bbe3f92e7d46c6e29ee4a1088fb. Closing this issue for the agreed typed-dispatch scope; the original proposal remains above for context.

    Delivered:

    • Typed source reading and migration dispatch, removing boxing from that dispatch path. The measured untracked struct scenarios allocate 48 B less per migration. Tracked structs still use boxing to preserve the tracking mutation correctly.
    • Per-call DI resolution remains intact for scoped/transient migrators. Without a service provider, the fallback path skips GetService.
    • Correct migration tracking for struct targets, plus struct and 1–8-source benchmark scenarios, revision-correct benchmark execution, and stronger behavioral coverage.
    • The final pre-merge snapshot passed 895 tests across .NET 10 and .NET 11, with a warning-free Release build and passing CI.

    Decisions on the remaining proposals:

    • The tracking-capability cache was intentionally omitted: shared reference-type generic code introduces a generic-static helper call, so it is not a universal improvement.
    • The Lazy<T> fallback remains. Its initialized path is already a volatile read plus a field read, and extra caching machinery was not justified.
    • The discriminator lookup change is deferred on egil/208-discriminator-lookup. Measurements showed a small, workload-dependent tradeoff, so the linear scan remains.
    • A full clean-machine refresh of docs/perf/*.md was not completed. The issue-specific measurement reports remain excluded as development artifacts; reusable benchmarks and scripts shipped.

    The 100% core branch coverage requirement is still unresolved and is now tracked separately in #249. At the final verified pre-merge snapshot, overall production branch coverage was 654/671 on .NET 10 and 904/919 on .NET 11. Those aggregate results do not satisfy the core-component policy, and closing this issue does not waive it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requeststjmEgil.SystemTextJson.Migration scope

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions