Skip to content

Refactor profile input loading responsibilities - #58

Merged
rogeralsing merged 1 commit into
mainfrom
codex/20260321-092243-69be633c
Mar 21, 2026
Merged

rogeralsing merged 1 commit into
mainfrom
codex/20260321-092243-69be633c

Conversation

@rogeralsing

Copy link
Copy Markdown
Contributor

Summary

  • split ProfileInputLoader into focused collaborators for trace input handling, heap input handling, input kind resolution, default mode policy, label generation, and memory profile result creation
  • updated execution/request flow to use the extracted helpers instead of loader-owned static utilities
  • kept the existing heap/input behavior covered by the test suite while reducing the main loader from a 300+ line mixed-responsibility class to a thin facade

Testing

  • roslynator fix src/ProfilerCore/Asynkron.Profiler.Core.csproj
  • roslynator fix src/ProfileTool/ProfileTool.csproj
  • roslynator fix tests/Asynkron.Profiler.Tests/Asynkron.Profiler.Tests.csproj
  • dotnet build Asynkron.Profiler.sln -warnaserror
  • dotnet test Asynkron.Profiler.sln
  • quickdup -path src -ext .cs -select 0..20 -min 2 -exclude ".g.,.generated."
  • dotnet format Asynkron.Profiler.sln

Notes

  • roslynator fix Asynkron.Profiler.sln hits the current CLI solution-parser issue in this environment, so Roslynator was run project-by-project instead.

Copilot AI review requested due to automatic review settings March 21, 2026 09:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors profile input loading in ProfileTool by extracting trace/heap input handling and shared “static utility” responsibilities (kind resolution, defaults policy, label generation, and memory result creation) into focused collaborators, leaving ProfileInputLoader as a thin facade.

Changes:

  • Extracted trace and heap input loaders (TraceProfileInputLoader, HeapProfileInputLoader) and updated ProfileInputLoader to delegate to them.
  • Centralized input kind resolution, default mode selection, label creation, and memory result creation into dedicated helpers.
  • Updated call sites and tests to use the extracted helpers.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Asynkron.Profiler.Tests/ProfileInputLoaderTests.cs Updated tests to use new factories/policy instead of ProfileInputLoader static helpers.
src/ProfileTool/ProfilerExecutionRequestFactory.cs Switched label/default-mode selection to new helpers when --input is provided.
src/ProfileTool/ProfileInputLoader.cs Reduced to a facade that composes and delegates to the new input loaders.
src/ProfileTool/ProfileCollectionRunner.cs Switched memory result construction to MemoryProfileResultFactory.
src/ProfileTool/Input/TraceProfileInputLoader.cs New: handles trace/speedscope input validation and analysis with consistent error reporting.
src/ProfileTool/Input/HeapProfileInputLoader.cs New: handles heap input kinds (.gcdump vs report files) and tool availability.
src/ProfileTool/Input/ProfileInputLabelFactory.cs New: builds sanitized input labels from file names.
src/ProfileTool/Input/ProfileInputKindResolver.cs New: resolves ProfileInputKind from file extensions and identifies trace inputs.
src/ProfileTool/Input/ProfileInputKind.cs New: enum representing supported input kinds.
src/ProfileTool/Input/ProfileInputDefaultsPolicy.cs New: applies default run-mode flags based on resolved input kind.
src/ProfileTool/Input/MemoryProfileResultFactory.cs New: constructs MemoryProfileResult from allocation call trees (sort + cap to 50).
Comments suppressed due to low confidence (1)

src/ProfileTool/ProfilerExecutionRequestFactory.cs:52

  • ProfileInputDefaultsPolicy.Apply is invoked with runCpu/runMemory potentially already true (because they’re initialized as invocation.* || !hasExplicitModes). Since the policy only turns flags on and never clears them, inputs like .etlx/.gcdump will still end up running CPU/memory by default, which contradicts the defaults asserted in ProfileInputLoaderTests.ApplyInputDefaults_MapsExtensionsToExpectedModes and can cause spurious “Unsupported CPU input” errors for heap-only inputs. Consider resetting all run* flags to false before applying the policy when hasInput && !hasExplicitModes, or have ProfileInputDefaultsPolicy.Apply explicitly set all flags based on the resolved input kind (clearing those not applicable).
            if (!hasExplicitModes)
            {
                ProfileInputDefaultsPolicy.Apply(
                    invocation.InputPath!,
                    ref runCpu,
                    ref runMemory,
                    ref runHeap,
                    ref runException,
                    ref runContention);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rogeralsing
rogeralsing merged commit 3ae00f1 into main Mar 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants