Skip to content

PERF-17/PERF-18 · Simplify ProcessorFactory and repository construction (remove reflection/double lookups) #435

Description

@LarsLaskowski

Finding: PERF-17 (Low) + PERF-18 (Low) · Area: F1Server.Service / F1Server.Db
Source: docs/concepts/udp-packet-processing-performance-review.md (branch claude/udp-packet-processing-review-798u7o), section 2.3

Files involved

  • F1Server.Service/Processors/ProcessorFactory.cs:113-375 — nine near-identical Get{X}Processor methods, each doing ContainsKey + TryGetValue (double hash lookup per packet) and creating via Activator.CreateInstance
  • F1Server.Db/Entity/RepositoryFactory.cs:92-102GetRepository<T> constructs repositories via Activator.CreateInstance
  • F1Server.Db/Entity/Repositories/Base/RepositoryBase{TQueryable,TEntity}.cs:65-68GetQuery() uses Activator.CreateInstance per query

Problem / Impact

Small but constant reflection and double-lookup overhead multiplied by per-packet/per-query call frequency, plus ~250 lines of copy-paste in ProcessorFactory.

Step-by-step fix

  1. ProcessorFactory: replace the nine Get{X}Processor methods with one private generic helper:
    private TProcessor GetOrCreateProcessor<TProcessor>(Func<TProcessor> factory)
        where TProcessor : BaseProcessor
    {
        return (TProcessor)_processors.GetOrAdd(typeof(TProcessor), _ => factory());
    }
    and call it from the switch in GetProcessor with direct constructors, e.g. PacketTypes.Session => GetOrCreateProcessor(() => new SessionProcessor(_serviceProvider, packetHeader, gameInfo)). Behavior note: RemoveAllProcessors() on session change and the frame-identifier logic in GetProcessor stay exactly as they are.
  2. RepositoryFactory.GetRepository: cache compiled constructor delegates in a private static readonly ConcurrentDictionary<Type, Func<F1ServerDbContext, RepositoryBase>>, built once per repository type via Expression.New + Expression.Lambda. The per-instance _repositories dictionary stays (same semantics), only the creation path changes.
  3. GetQuery(): same approach — a static compiled delegate per TQueryable type (Func<IQueryable<TEntity>, TQueryable>), replacing Activator.CreateInstance(typeof(TQueryable), ...).
  4. Note: all affected classes are internal/public with known constructor signatures — if a constructor signature does not match at delegate-build time, fail fast with a clear exception message.

Test guidance

Existing processor and repository tests cover the behavior; they must stay green unchanged. Add one test asserting ProcessorFactory.GetProcessor returns the same instance for two packets of the same type and session, and a new instance after a session-id change (this pins the caching semantics the refactor must preserve).

Acceptance criteria

  • No Activator.CreateInstance in the per-packet or per-query path
  • ProcessorFactory caching implemented once (generic helper), nine copies removed
  • Processor-caching semantics pinned by a test (same session → same instance; new session → fresh instances)
  • Full rebuild shows 0 Reihitsu (RH####) warnings

Related: #420 (long-lived factories reduce how often the repository delegates run — still worth removing the reflection).

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancePerformance improvements

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions