Context switch samples were appended in arrival order. Samples which
were postponed due to missing context switch data are replayed after
newer samples were already classified, so the vector could become
unordered. Everything that reads it assumes time order: the wait stacks
range filter, the sampling statistics percentage denominator, and the
context switch sample filters in the trace load jobs. In the load jobs
an unordered vector could silently disable the filtering for the rest
of a thread, reintroducing the context switch samples into the symbol
and child sample maps.
Use a SortedVector and restore the ordering at the points where it can
break: after the postponed sample replay and when saving a trace. The
save file version is bumped, so that the sort order check on load is
only performed for traces saved by previous versions.
During live capture, child samples were appended to their per-address
vectors in arrival order. Samples postponed due to missing context
switch data are replayed after newer samples were already processed, so
the vectors could become unordered. All range-limited queries binary
search these vectors by time and would silently return wrong results.
Inserting in sorted order at collection time would require a mid-vector
insertion for every stack frame of every replayed sample, so instead
the vectors are now SortedVector and are sorted lazily when accessed,
following what the inline symbol list already does. This also restores
proper query results for traces with inconsistent sample order.
Context switch samples are excluded from sampling statistics, as they
are not produced by the statistical profiling timer and their stacks
are always parked at the scheduler. During live capture this exclusion
is structural, as such samples never enter the statistics processing
path. On trace load, however, only the job which computes symbol
statistics and the instruction pointer map filtered them out. The job
which builds the per-symbol sample lists and the child sample map did
not, so on a loaded trace these two structures included tens of
thousands of context switch samples that a live session would not
count.
This made range-limited statistics counts exceed the whole-trace
counts, inflated child sample costs in the symbol view, and caused the
same trace to show different numbers live and after a save and reload.
On the test trace, __schedule reported 56 exclusive samples but 32747
entries in its sample list.
Apply the same context switch sample filter when building the symbol
samples and child samples maps.
SymbolStats now has two additional entry stack maps, which record the
call stacks below every occurrence of a symbol in a sample, not only
when the symbol was at the top of the stack:
- wasReached counts each occurrence separately. If a symbol re-enters
itself through recursion, every re-entry adds an entry stack, which
will itself contain the symbol somewhere below.
- wasReachedNonReentrant counts only the outermost occurrence, so each
sample contributes exactly once and recursion is ignored. The sum of
counts in this map equals the symbol's inclusive sample count.
To achieve this, the call stack is walked bottom-up, which makes the
first encountered occurrence of a symbol the outermost one. Symbols at
inline positions get the remaining inline frames of their frame group
as a synthetic frame, mirroring what was already done for the top of
the stack.
The wasExecuting and wasExecutingBase maps are now filled by the same
walk, as its topmost-frame special case. The keys they receive are
identical to what the previous code produced.
Note that there is no "base" variant of the reached maps. A base symbol
is present as the last frame of every frame group that contains its
inline functions, so its wasReached map already covers the whole symbol
at base granularity. Merging in the inline symbols' maps, as done for
wasExecutingBase, would only multi-count the same samples.
The current "parents" mode of operation is "was this symbol executing".
The new name for the maps reflects that.
There should also be "was this symbol reached" (both recursive and non-
reentrant) modes.
Cache is shared between image names and source file names, because the
underlying StringIdx storage makes indices unique. Both name sets should
be completely separate, but if you have conflicts here, you have much
more pressing problems to solve.
Frames whose symbol data is shipped inline with the callstack payload
(sel=1, e.g. Lua-side stack entries) were being passed to
GetCanonicalPointer() in the AddCallstackAllocPayload() query loop,
tripping its sel==0 assertion. They have no native pointer to query
and were already registered in callstackFrameMap earlier in the same
function, so just skip them.
Regression from c704f909, which hoisted the per-call-site dedup into
QueryCallstackFrame(). Three of the four updated call sites were
equivalent before and after, because the old guard and the new one
keyed on the same value. The fourth, this one, was not: the old guard
tested the frame as-is and matched the entry inserted a few lines above,
short-circuiting before GetCanonicalPointer() ran. The new guard keys on
PackPointer(addr), so GetCanonicalPointer() must run first to compute
addr, and the assert fires.
The profiler will typically want to send bursts of queries (e.g. 3 queries
to retrieve source location strings, or multiple queries to get all the call
stack frames, etc.).
Each of these queries will be sent immediately, if available space in the
network buffer permits. Each of these sends is a separate syscall.
Remove this and instead batch all queries with the already existing network
buffer overflow handling functionality.
There are two changes to the protocol:
- `QueueMessageLiteral*` were changed and what used to be addresses are now addresses+metadata
- Other messages now send `QueueMessage*Metadata` with added metadata.
This will later be used to store and transmit message sources, level, etc.
This was causing issues in the Infos -> Trace Statistics window as `GetCallstackFrameCount` uses `m_pendingCallstackFrames`. Just in case, init those all those variables where declared instead of constructor.
This provides some instructions and tips for the manual. Also:
* Made the calibration feature a CMake option
* Cleaned up some minor code issues
* Fixed an issue with the calibration
* Incremented patch number
Callstack frames will now have nullptr as the value in the callstackFrameMap
map, as a way to signal that a query for given key is already pending.
Duplicate queries should no longer happen.
@slomp provided alternative implementation, which produced the following
results:
Queries made: 195,778
Duplicate queries skipped: 9,518,910
Co-authored-by: Marcos Slomp <slomp@adobe.com>
Bartosz Taudul
Clément Grégoire
Trevor L. McDonell
Antoine Mura
Eric Eaton
