Refactor profile input loading responsibilities - #59
Closed
rogeralsing wants to merge 1 commit into
Closed
rogeralsing wants to merge 1 commit into
rogeralsing wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors profile-input handling by centralizing input-path classification/labeling/default-mode selection and extracting memory-profile shaping logic, while simplifying the loader and splitting tests accordingly.
Changes:
- Introduce
ProfileInputPath+ProfileInputKindto handle input kind detection, label building, trace detection, and default-mode selection. - Extract allocation call-tree →
MemoryProfileResultshaping intoMemoryProfileResultFactory. - Update callers and reorganize tests into focused suites (
ProfileInputPathTests,MemoryProfileResultFactoryTests).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Asynkron.Profiler.Tests/ProfileInputPathTests.cs | Adds unit tests for input kind detection, label fallback, and default mode mapping. |
| tests/Asynkron.Profiler.Tests/ProfileInputLoaderTests.cs | Removes tests moved to the new focused suites. |
| tests/Asynkron.Profiler.Tests/MemoryProfileResultFactoryTests.cs | Adds unit tests for memory result shaping/sorting/capping behavior. |
| src/ProfileTool/ProfilerExecutionRequestFactory.cs | Switches label/default-mode logic to ProfileInputPath. |
| src/ProfileTool/ProfileInputPath.cs | New central implementation for label/kind/default-mode logic. |
| src/ProfileTool/ProfileInputLoader.cs | Simplifies loader by delegating kind detection and memory result shaping. |
| src/ProfileTool/ProfileInputKind.cs | New enum defining supported input kinds. |
| src/ProfileTool/ProfileCollectionRunner.cs | Uses MemoryProfileResultFactory for memory results. |
| src/ProfileTool/MemoryProfileResultFactory.cs | New extracted factory for producing MemoryProfileResult from allocation call trees. |
Comments suppressed due to low confidence (1)
src/ProfileTool/ProfilerExecutionRequestFactory.cs:53
ApplyDefaultsonly ever sets flags totrue. In this block,runCpu/runMemoryare initialized totruewhen no explicit modes are provided (invocation.* || !hasExplicitModes), so the extension-based defaults can’t actually turn modes off. This makes--inputdefault mode selection ineffective (e.g., a.gcdumpinput can still try to run CPU/memory). Consider re-initializing allrun*flags tofalsewhenhasInput && !hasExplicitModesbefore callingApplyDefaults(and then re-apply the Hot/JIT forcing logic), or changeApplyDefaultsto explicitly assign all flags for each kind.
if (!hasExplicitModes)
{
ProfileInputPath.ApplyDefaults(
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ProfileInputPathandProfileInputKindMemoryProfileResultFactoryProfileInputLoaderso it focuses on file existence checks, heap/trace orchestration, and themed error reportingTesting
roslynator fix src/ProfilerCore/Asynkron.Profiler.Core.csprojroslynator fix src/ProfileTool/ProfileTool.csprojroslynator fix tests/Asynkron.Profiler.Tests/Asynkron.Profiler.Tests.csprojdotnet format Asynkron.Profiler.sln --verify-no-changesdotnet build Asynkron.Profiler.sln -v minimaldotnet test Asynkron.Profiler.sln -v minimalquickdup -path . -ext .cs -select 0..30 -min 2 -exclude ".g.,.generated.,bin/,obj/"