Refactor profiling engine: Migrate aggregation and call tree processi…#62
Draft
trgibeau wants to merge 1 commit into
Draft
Refactor profiling engine: Migrate aggregation and call tree processi…#62trgibeau wants to merge 1 commit into
trgibeau wants to merge 1 commit into
Conversation
…ng to FunctionProfiler - Moved call tree processing logic from CallTreeProcessor to ProfileData.ComputeProfile. - Removed FunctionProfileProcessor and integrated its functionality into ProfileData. - Introduced FunctionProfiler for handling resolved input and aggregation. - Updated ProfileData to utilize concurrent collections for thread-safe operations. - Added tests for new aggregation logic and ensured parity with previous implementations. - Created new tests for ProfileReportMapper and RawProfileLibraryAdapter to validate functionality.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Profile Explorer’s profiling engine pipeline so that aggregation and call-tree construction are owned by the ProfileExplorer.Profiling library (via FunctionProfiler), while Core focuses on symbol/stack resolution and projecting resolved stacks into the library’s neutral model.
Changes:
- Migrate Core’s per-function aggregation + call tree building from
FunctionProfileProcessor/CallTreeProcessorinto the library (FunctionProfiler,SampleAggregator,CallTreeBuilder) and updateProfileData.ComputeProfileto use the new resolved-input path. - Introduce adapter/mapping layer (
RawProfileLibraryAdapter,ProfileReportMapper) to bridge Core’s ETW/raw + UI-facingProfileDatamodel to/from the library’s neutral abstractions. - Add parity/characterization tests to validate aggregation semantics, instance-path focusing, and the Core↔library mapping layer.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ProfileExplorerCoreTests/RawProfileLibraryAdapterTests.cs | New tests validating ETW/raw→library adapter behavior (PDB identity + leaf-first stacks). |
| src/ProfileExplorerCoreTests/ProfileReportMapperTests.cs | New tests validating library report→Core ProfileData mapping (profiles/resolver/module weights/totals). |
| src/ProfileExplorerCoreTests/ETWUnmappedFrameResolutionTests.cs | Updates processor tests to target the new ComputeProfile (library-backed) path. |
| src/ProfileExplorerCoreTests/EngineAggregationParityTests.cs | New cross-engine parity tests for aggregation semantics and parallel determinism. |
| src/ProfileExplorerCore/Profile/Processing/FunctionProfileProcessor.cs | Removed legacy Core aggregation processor. |
| src/ProfileExplorerCore/Profile/Processing/CallTreeProcessor.cs | Removed legacy Core call-tree processor. |
| src/ProfileExplorerCore/Profile/Data/ProfileData.cs | Reworked ComputeProfile to project resolved stacks into the library and apply the returned report. |
| src/ProfileExplorerCore/Profile/Adapters/RawProfileLibraryAdapters.cs | New Core→library input adapters for raw ETW-derived images/samples. |
| src/ProfileExplorerCore/Profile/Adapters/ProfileReportMapper.cs | New mapper projecting ProfileReport back into Core’s ProfileData shape. |
| src/ProfileExplorer.Profiling/Profiling/SampleAggregator.cs | Adds resolved-stack aggregation + instance-path filtering + caller per-instruction attribution. |
| src/ProfileExplorer.Profiling/Profiling/IpResolver.cs | Adds instance-path prefix checks for raw and pre-resolved stacks. |
| src/ProfileExplorer.Profiling/Profiling/CallTreeBuilder.cs | Adds instance-path filtering and a parallel chunk-tree build/merge path for resolved stacks. |
| src/ProfileExplorer.Profiling/ProfileExplorer.Profiling.csproj | Exposes internals to Core tests for cross-engine parity testing. |
| src/ProfileExplorer.Profiling/FunctionProfiler.cs | Adds resolved-input mode, host symbol injection, instance-path filtering, and parallel resolved aggregation API. |
| src/ProfileExplorer.Profiling/Abstractions/ResolvedSampleProjector.cs | New delegate contract for parallel host projection into neutral resolved frames. |
| src/ProfileExplorer.Profiling/Abstractions/ResolvedFrame.cs | New neutral resolved-frame model for host-owned symbol resolution. |
| src/ProfileExplorer.Profiling.Tests/Unit/FunctionProfilerInjectionTests.cs | New tests for host symbol injection and instance-path focusing. |
| src/ProfileExplorer.Profiling.Tests/Unit/AggregatorSemanticsCharacterizationTests.cs | New characterization test ensuring library per-instruction attribution matches Core semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -37,6 +42,13 @@ public void AddSamples(IEnumerable<IProfileSample> samples) { | |||
| foreach (var sample in samples) { | |||
| if (string.IsNullOrEmpty(sample.ImageName)) continue; | |||
Comment on lines
+352
to
+357
| functionResolver.TryAdd(details.FunctionId, details.Function); | ||
| } | ||
| } | ||
|
|
||
| return frames.Count > 0; | ||
| } |
|
|
||
| <ItemGroup> | ||
| <InternalsVisibleTo Include="ProfileExplorer.Profiling.Tests" /> | ||
| <!-- Cross-engine parity tests drive the internal SampleAggregator alongside Core's FunctionProfileProcessor. --> |
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.
…ng to FunctionProfiler