[PROF-14955] Skip expensive SymFromAddr for modules without PDB symbols#58
[PROF-14955] Skip expensive SymFromAddr for modules without PDB symbols#58r1viollet wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4922f08dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // SymFromAddr failed - mark module as having failed symbol load to avoid expensive retries | ||
| if (moduleCacheKey != 0) { | ||
| auto it = _moduleCache.find(moduleCacheKey); | ||
| if (it != _moduleCache.end()) { | ||
| it->second.symbolLoadFailed = true; |
There was a problem hiding this comment.
This marks the whole module as failed after any SymFromAddr miss, but a miss can be address-specific (for example an address in a gap/header region or otherwise outside any loaded symbol) while other addresses in the same module still resolve. Once this flag is set, the early return above skips SymFromAddr for every subsequent address in that module until RefreshModules, so one unsymbolizable frame can turn valid symbols for the rest of the module into empty function names. Please gate the cache on a confirmed module symbol-load failure (e.g. module state/GetLastError) rather than every lookup failure.
Useful? React with 👍 / 👎.
r1viollet
changed the title
bb0e902 to
2ca0ad9
Compare
CPU profile showed SymFromAddr burning ~27% of CPU time via: SymFromAddr -> SymLoadModuleExW -> SymFindFileInPath -> GetSymLoadError SymFromAddr succeeds (returning export symbols for ntdll, kernel32, etc.) but internally re-attempts PDB loading on every call. Two fixes: 1. Remove SYMOPT_DEFERRED_LOADS so symbols load upfront at SymInitialize instead of triggering expensive deferred loads on every SymFromAddr. 2. Track per-module whether full symbols (PDB/DIA) are available. After the first successful SymFromAddr for a module, check SymType. If only exports, skip SymFromAddr for subsequent addresses. The module-level flag is only set on SymFromAddr success (where SymType is reliable). A per-address failure (gap/header region) does not poison the module cache.
2ca0ad9 to
3b7b7c9
Compare
|
not a huge priority for now (as symbolication is not expected to be triggered on profiler side in many use cases) |
1 participant
Description
Skip repeated expensive DbgHelp calls for modules without PDB symbols.
Motivation
CPU profile of the profiler showed
SymFromAddrburning ~27% of CPU via:SymFromAddrsucceeds (returning export symbols for ntdll, kernel32, etc.) but internally re-attempts PDB loading on every call. WithSYMOPT_DEFERRED_LOADS, DbgHelp loads and unloads module symbols on each invocation.Changes
SYMOPT_DEFERRED_LOADS— symbols load once atSymInitializeinstead of triggering expensive deferred loads on everySymFromAddr.SymFromAddrfor a module, checkSymType. If only exports, skipSymFromAddrfor subsequent addresses in that module.dd-win-prof.dlltimestamp incapture-pprof.ps1.ScopedHandle→CloseHandleon garbage).Testing
TestModuleLevelFailureCachingChecklist