Associate label sets with wall profiles#105
Merged
Conversation
Overall package sizeSelf size: 2.08 MB Dependency sizes
🤖 This report was automatically generated by heaviest-objects-in-the-universe |
e38f05e to
6009fcf
Compare
…le with labels was taken
a11552d to
522d443
Compare
r1viollet
reviewed
Jun 28, 2023
| #endif | ||
|
|
||
| if (includeLines && withLabels) { | ||
| // Currently custom labels are not compatible with caller line |
There was a problem hiding this comment.
great comment, should we add a ticket to v8 for this ?
r1viollet
reviewed
Jun 28, 2023
r1viollet
reviewed
Jun 28, 2023
48fe3e5 to
dfbf67b
Compare
Make line number incompatible with custom labels
dfbf67b to
516a9d6
Compare
theanarkh
reviewed
Aug 5, 2023
| // skip first sample because it's the one taken on profiler start, outside of | ||
| // signal handler | ||
| for (int i = 1; i < sampleCount; i++) { | ||
| // Handle out-of-order samples, hypothesis is that at most 2 consecutive |
There was a problem hiding this comment.
@nsavoire Hi, I want to know why this happened and why do you need to deal with this situation ? I read the V8 source code and found that the timestamp of samples is incremental (test/cctest/test-cpu-profiler.cc).
uint64_t end_time = profile->GetEndTime();
uint64_t current_time = profile->GetStartTime();
CHECK_LE(current_time, end_time);
for (int i = 0; i < profile->GetSamplesCount(); i++) {
CHECK(profile->GetSample(i));
uint64_t timestamp = profile->GetSampleTimestamp(i);
// <=
CHECK_LE(current_time, timestamp);
CHECK_LE(timestamp, end_time);
current_time = timestamp;
}
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.
Based on #98
Adds ability to associate custom label sets to wall samples. This can then be used by dd-trace-js to associate span ids with said samples.
Implementation notes
The implementation adds JS API to set and clear labels. In practice, the
WallProfilerJS class will gain getters and setters for propertieslabels.Internally, the implementation uses a PPROF signal handler executing before V8's signal handler used for profiling. It will associate the current label set with a timestamp, and put it in a preallocated buffer (so allocations are avoided in the signal handler.) This happens once every 10ms while profiling. The buffer is sized generously, for 2x the number of samples for a single profile.
When the profile is post-processed once every minute, the labels are matched to samples based on the timestamp, and emitted into the serialized profile.
Timestamp matching
Two timestamps are collected in profiler signal handler and are associated with the current label: one before v8 handler is invoked and one after.
These two timestamps allow easily matching v8 profiling samples to labels, each sample is matched with the label whose timestamp interval contains the sample timestamp.
v8 does not expose publicly its clock API (
v8::base::TimeTicks::Now()), matching samples / labels without a common clock source proved to be difficult, hence profiler uses private v8 API by redeclaring it.