Skip to content

Move refactor profiling engine - #65

Merged
trgibeau merged 12 commits into
mainfrom
user/trgibeau/profilingLib2
Jul 28, 2026
Merged

Move refactor profiling engine#65
trgibeau merged 12 commits into
mainfrom
user/trgibeau/profilingLib2

Conversation

@trgibeau

Copy link
Copy Markdown
Collaborator

This pull request introduces a set of new unit tests and abstractions to improve and validate the profiling library's support for host-driven symbol resolution and stack aggregation. It also adds a new construction path for FunctionProfiler to enable use with pre-resolved input (bypassing symbol download and validation). The most important changes are:

New and Improved Unit Tests

  • Added AggregatorSemanticsCharacterizationTests to verify that the library's aggregator matches the core engine's per-instruction attribution semantics, ensuring behavior is preserved when delegating to the library.
  • Added FunctionProfilerInjectionTests to test host-driven symbol injection, including symbol resolution bypass, argument validation, and focused stack aggregation using instance paths.

Abstractions for Host-Resolved Input

  • Introduced the ResolvedFrame record struct, representing a pre-resolved stack frame provided by a host, including function identity, debug info, and instruction offset calculation.
  • Added the ResolvedSampleProjector delegate abstraction, allowing hosts to project their sample data into the library's neutral resolved form for parallel aggregation.

Construction and API Enhancements

  • Added FunctionProfiler.CreateForResolvedInput() static method and internal constructor overload to enable profiling with pre-resolved stacks, skipping symbol download and option validation.

trgibeau added 4 commits July 8, 2026 15:46
…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.

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

This PR advances the “profiling engine deduplication” effort by shifting Core’s per-sample aggregation and call-tree building onto the ProfileExplorer.Profiling library, while adding adapter layers and targeted unit tests to validate parity and host-driven resolved-input scenarios.

Changes:

  • Replaced Core’s FunctionProfileProcessor / CallTreeProcessor aggregation path with a library-driven resolved-input aggregation pipeline (FunctionProfiler.CreateForResolvedInput + parallel projection).
  • Added Core↔library adapter layers for ETW raw input (RawProfileLibraryAdapter) and library output→Core model projection (ProfileReportMapper).
  • Improved DIA activation robustness (registration-free side-load fallback) and added extensive parity/characterization/injection tests across Core tests and Profiling library tests.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/ProfileExplorerCoreTests/RawProfileLibraryAdapterTests.cs Adds unit coverage for raw ETW→library adapter behavior (PDB identity + leaf-first stacks).
src/ProfileExplorerCoreTests/ProfileReportMapperTests.cs Adds tests for mapping library ProfileReport back into Core ProfileData.
src/ProfileExplorerCoreTests/ETWUnmappedFrameResolutionTests.cs Updates processor tests to use the new ComputeProfile path.
src/ProfileExplorerCoreTests/EngineAggregationParityTests.cs Adds parity/regression tests for aggregation semantics and parallel vs sequential results.
src/ProfileExplorerCore/Session/BaseSession.cs Ensures binaries are lazily loaded before disassembly in headless/ETW flows.
src/ProfileExplorerCore/Profile/Processing/FunctionProfileProcessor.cs Deletes Core per-function aggregation processor (superseded by library).
src/ProfileExplorerCore/Profile/Processing/CallTreeProcessor.cs Deletes Core call-tree processor (superseded by library).
src/ProfileExplorerCore/Profile/Data/ProfileData.cs Reimplements ComputeProfile using library aggregation over resolved stacks + report mapping.
src/ProfileExplorerCore/Profile/Adapters/RawProfileLibraryAdapters.cs Introduces ETW raw model → library IProfileImage/IProfileSample adapters.
src/ProfileExplorerCore/Profile/Adapters/ProfileReportMapper.cs Introduces library report → Core ProfileData projection.
src/ProfileExplorerCore/Binary/PDBDebugInfoProvider.cs Switches DIA activation to a side-load-first path shared with the library.
src/ProfileExplorer.Profiling/Symbols/PdbSymbolProvider.cs Stabilizes function ordering (ICF tie-break) and exposes DIA data source factory.
src/ProfileExplorer.Profiling/Profiling/SampleAggregator.cs Adds instance-path filtering + resolved-frame aggregation support + per-instruction caller attribution parity.
src/ProfileExplorer.Profiling/Profiling/IpResolver.cs Adds instance-path prefix checks for raw and resolved stacks.
src/ProfileExplorer.Profiling/Profiling/CallTreeBuilder.cs Adds instance filtering + resolved-stack add + parallel chunk-tree merge support.
src/ProfileExplorer.Profiling/ProfileExplorer.Profiling.csproj Exposes internals to ProfileExplorerCoreTests for parity tests.
src/ProfileExplorer.Profiling/FunctionProfiler.cs Adds resolved-input construction + resolved-sample APIs + parallel resolved aggregation driver.
src/ProfileExplorer.Profiling/Abstractions/ResolvedSampleProjector.cs Adds host projection delegate abstraction for parallel resolved aggregation.
src/ProfileExplorer.Profiling/Abstractions/ResolvedFrame.cs Adds neutral resolved-frame record struct for host-provided frames.
src/ProfileExplorer.Profiling.Tests/Unit/FunctionProfilerInjectionTests.cs Adds tests for host symbol injection and instance-path focusing behavior.
src/ProfileExplorer.Profiling.Tests/Unit/AggregatorSemanticsCharacterizationTests.cs Adds characterization test ensuring per-instruction attribution matches Core semantics.
Comments suppressed due to low confidence (1)

src/ProfileExplorerCore/Profile/Adapters/RawProfileLibraryAdapters.cs:107

  • stackFrames is allowed to be null (see CreateSamples and RawProfileLibraryAdapterTests), but the constructor/field are non-nullable. Make it nullable to align with the IProfileSample.StackFrames contract and avoid nullable warnings.
public sealed class RawProfileSampleAdapter : IProfileSample {
  private readonly long[] stackFrames_;

  public RawProfileSampleAdapter(long ip, TimeSpan weight, int processId, int threadId,
                                 string? imageName, long imageBaseAddress, long[] stackFrames) {

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

Comment thread src/ProfileExplorerCore/Profile/Data/ProfileData.cs
Comment thread src/ProfileExplorerCore/Profile/Data/ProfileData.cs
Comment thread src/ProfileExplorerCore/Profile/Adapters/ProfileReportMapper.cs
Comment thread src/ProfileExplorerCore/Profile/Adapters/ProfileReportMapper.cs
Comment thread src/ProfileExplorerCore/Profile/Adapters/RawProfileLibraryAdapters.cs Outdated
@tgibeau

tgibeau commented Jul 23, 2026

Copy link
Copy Markdown

@copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

@trgibeau
trgibeau marked this pull request as ready for review July 27, 2026 19:10
Comment thread src/ProfileExplorer.Profiling/Profiling/CallTreeBuilder.cs Outdated
@trgibeau
trgibeau merged commit c6bed33 into main Jul 28, 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.

5 participants