Skip to content

perf: command discovery scans the whole AppDomain with the materializing attribute API, and it runs on the first frame in the editor #36

Description

@wallstop

Filed from a consumer: Ambiguous-Interactive/IshoBoy, package com.wallstop-studios.dxcommandterminal@1.0.0-rc25.0. Downstream issue: https://github.com/Ambiguous-Interactive/IshoBoy/issues/796.

Symptom in the consumer

Entering Play Mode on the main menu takes roughly 30 seconds, after the domain reload has already finished. The terminal prefab is spawned from a dev-only child spawner on that scene, synchronously, inside Start().

The code

Runtime/CommandTerminal/Backend/CommandShell.cs:17-88RegisteredCommands is a Lazy<> whose factory is:

Type type in AppDomain
    .CurrentDomain.GetAssemblies()          // :28
    .Except(ourAssembly)
    .Concat(ourAssembly)
    .SelectMany(assembly => assembly.GetTypes())   // :35
foreach (MethodInfo method in type.GetMethods(methodFlags))   // :40  Static|Public|NonPublic
    Attribute.GetCustomAttribute(method, typeof(RegisterCommandAttribute))   // :45

It is forced from TerminalUI.OnEnable -> RefreshStaticState -> InitializeAutoRegisteredCommands (Runtime/CommandTerminal/UI/TerminalUI.cs:340-342, :410, :465; CommandShell.cs:175), i.e. the frame the terminal is instantiated, on the main thread.

Why it is much worse in the editor than in a build

AppDomain.CurrentDomain in editor Play Mode is the editor domain, not the player's. In this consumer that is 500+ assemblies: UnityEditor, the whole of com.unity.ai.assistant, Rider, Roslyn, TMP, Animancer, Cinemachine, the test frameworks. That is on the order of 10^5-10^6 methods walked to find, in this project, 7 methods in one file.

The IgnoredTypes = { "JetBrains.Rider" } list at :14 is, I think, the same problem already noticed from the exception side: the sweep is reaching assemblies it has no business in.

Two independent fixes, both cheap

1. Filter by assembly reference before touching types. A RegisterCommandAttribute can only appear in an assembly that references the terminal's own assembly. That is readable without loading types:

AssemblyName self = typeof(BuiltInCommands).Assembly.GetName();

static bool MayContainCommands(Assembly assembly, AssemblyName self)
{
    if (assembly.IsDynamic) return false;
    if (AssemblyName.ReferenceMatchesDefinition(assembly.GetName(), self)) return true;
    foreach (AssemblyName referenced in assembly.GetReferencedAssemblies())
    {
        if (AssemblyName.ReferenceMatchesDefinition(referenced, self)) return true;
    }
    return false;
}

GetReferencedAssemblies() reads the metadata table and does not force type loading, so this removes the great majority of assemblies before GetTypes() is ever called. It also makes IgnoredTypes unnecessary for its original purpose, since Rider's assemblies do not reference the terminal.

2. Use IsDefined as the filter and materialize only the survivors. Attribute.GetCustomAttribute constructs the attribute instance and walks the inheritance chain for every method scanned. MethodInfo.IsDefined(Type, bool) answers the same question off the metadata:

if (!method.IsDefined(typeof(RegisterCommandAttribute), inherit: false)) continue;
if (Attribute.GetCustomAttribute(method, typeof(RegisterCommandAttribute))
    is not RegisterCommandAttribute attribute) continue;

The allocation then happens once per real command instead of once per method in the domain.

Both are behaviour-preserving. The .Except(ourAssembly).Concat(ourAssembly) ordering trick at :26-34, which exists so user commands win conflicts, is unaffected by either.

A third, if you want the frame back entirely

Even after the above, the work is still done on whatever frame the terminal first enables. Consumers that spawn the terminal at boot pay it at boot. Deferring the Lazy<> force to the first time the terminal is actually opened would move it off the startup path for everyone, at the cost of a small hitch on first open. Worth considering as a separate switch rather than a default change.

What I could not do

I could not attach a measurement. The consumer's Unity MCP bridge refuses System.Reflection in the snippets it will run, so timing the sweep in the live editor was not available to me, and I would rather file this without a number than with a fabricated one. The nearest hard datum from that project is a strict subset of this work -- an all-types, all-methods editor scan -- measured at 5448 ms (progress/session-005-menu-instant-load.md:32).

A System.Diagnostics.Stopwatch around the Lazy<> factory, logged at Debug, would both confirm this and be useful to keep.

Activity

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions