More Profile Points and Only Run Profiler in Main Thread#233
Merged
Conversation
…n logic; add unit tests for profiling functionality
- Insert IPC_TOOLKIT_PROFILE_BLOCKs in collision, candidate, and potential build and evaluation routines for detailed profiling - Add name() method to all Potential subclasses for block labeling - Extend profiler test to cover full IPC pipeline on sample mesh data - Update includes to ensure profiler header is available where needed
- Restrict Profiler to record only from the main thread for accurate timings - Add m_main_thread_id and m_current_scope to Profiler for thread tracking - Ignore profiling calls from non-main threads - Update tests to reflect single-threaded counting and aggregation - Refactor test to check counts and timings for nested blocks - Minor cleanup and improved comments
- Wrap local Hessian computation and mapping in IPC_TOOLKIT_PROFILE_BLOCK - Adjust profiler test to profile parallel region as a single block
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands profiling coverage across core IPC routines and refactors the profiler so it only records from a single “main” thread, aiming to provide finer-grained performance visibility without multi-threaded profiler state complexity.
Changes:
- Added
IPC_TOOLKIT_PROFILE_BLOCKinstrumentation to key IPC entry points, candidate generation, collision building, and potential evaluation/derivatives. - Refactored
Profilerstate management (m_current_scope) and added thread filtering (m_main_thread_id) so non-recording threads are ignored. - Introduced a new Catch2 profiler test suite and added
Potential::name()to label profiling scopes by potential type.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/tests/utils/test_profiler.cpp | Adds profiler-focused tests and a full-pipeline profiling run. |
| tests/src/tests/utils/CMakeLists.txt | Registers the new profiler test file in the utils test target. |
| src/ipc/utils/profiler.hpp | Adds thread support and stores main-thread id in profiler state. |
| src/ipc/utils/profiler.cpp | Implements main-thread-only recording, refactors clear/reset behavior, and updates scope tracking. |
| src/ipc/potentials/potential.hpp | Adds name() as a pure-virtual identifier for profiling scope labels. |
| src/ipc/potentials/potential.cpp | Adds profiling blocks around potential value/gradient/hessian computation and sub-steps. |
| src/ipc/potentials/tangential_adhesion_potential.hpp | Implements name() for profiling. |
| src/ipc/potentials/normal_adhesion_potential.hpp | Implements name() for profiling. |
| src/ipc/potentials/friction_potential.hpp | Implements name() for profiling. |
| src/ipc/potentials/barrier_potential.hpp | Implements name() for profiling. |
| src/ipc/ipc.cpp | Adds profiling blocks to IPC public entry points (step checks / step size / intersections). |
| src/ipc/collisions/tangential/tangential_collisions.cpp | Adds profiling blocks to tangential collision building. |
| src/ipc/collisions/normal/normal_collisions.cpp | Adds profiling blocks to normal collision building. |
| src/ipc/candidates/candidates.cpp | Adds profiling blocks to candidate building and step size computation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Add Profiler::is_recording_thread() to check current thread id - Make ProfilePoint store m_active and skip start/stop when not on the main thread
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
- Coverage 95.84% 95.82% -0.03%
==========================================
Files 160 161 +1
Lines 16589 16603 +14
Branches 919 918 -1
==========================================
+ Hits 15900 15909 +9
- Misses 689 694 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Description
This pull request adds profiling instrumentation throughout the codebase to enable fine-grained performance measurement. The profiler is also improved to be single-threaded, only recording data from the main thread, and its internal state management is refactored for clarity and correctness.
Type of change