Skip to content

[Profiler] Send only activated profilers values in profile#3337

Merged
chrisnas merged 4 commits into
masterfrom
chrisnas/enabled_profiler_only_values_in_pprof
Oct 12, 2022
Merged

[Profiler] Send only activated profilers values in profile#3337
chrisnas merged 4 commits into
masterfrom
chrisnas/enabled_profiler_only_values_in_pprof

Conversation

@chrisnas

Copy link
Copy Markdown
Contributor

Summary of changes

Send only activated profilers values in profile

Reason for change

For back end metrics computation, it is not possible to figure out which value is 0 because the corresponding profiler is disabled or no value. With this change, only values of activated profilers will be available in profiles.

Implementation details

Per profiler values offset is computed when the profilers are enabled. These offset are passed down to the collector base class to allow transformation of raw data into corresponding value slots in profiles.

Test coverage

Updated all test cases and adding dedicated ones for wall time

Other details

@chrisnas
chrisnas requested a review from a team as a code owner October 10, 2022 17:08
@github-actions github-actions Bot added the area:profiler Issues related to the continous-profiler label Oct 10, 2022

@gleocadie gleocadie left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

size_t Sample::ValuesCount = 16; // should be set BEFORE any sample gets created


//s

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed //s

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ done

Comment on lines +34 to +39
_values.reserve(ValuesCount);
for (size_t i = 0; i < ValuesCount; i++)
{
_values.push_back(0);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ thanks

@chrisnas
chrisnas merged commit fca62c0 into master Oct 12, 2022
@chrisnas
chrisnas deleted the chrisnas/enabled_profiler_only_values_in_pprof branch October 12, 2022 10:09
@github-actions github-actions Bot added this to the vNext milestone Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:profiler Issues related to the continous-profiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants