Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds timing info to each sample #133

Merged
merged 4 commits into from Jun 20, 2022

Conversation

viglia
Copy link
Contributor

@viglia viglia commented Jun 6, 2022

At the moment we only have timing info at Report level. We don't have more fine grained timing info for the samples collected.

This PR aims at adding timing info (sample_timestamp) to the samples collected.

@viglia viglia force-pushed the viglia/enhance/add-sample-timing-info branch from 0d0a125 to d8bd3bb Compare June 6, 2022 09:34
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

Good! As the clock_gettime is usually fast (with vDSO in Linux), I think it won't have too much performance degradation. I'll also bench it to make sure.

BTW, please modify the CHANGELOG.md

Rest LGTM

@viglia viglia force-pushed the viglia/enhance/add-sample-timing-info branch from 72659dd to 3ce1876 Compare June 9, 2022 08:56
Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>
Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>
Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>
@viglia viglia force-pushed the viglia/enhance/add-sample-timing-info branch from 3ce1876 to 32352a9 Compare June 9, 2022 08:57
@viglia
Copy link
Contributor Author

viglia commented Jun 9, 2022

@YangKeao thanks for looking over this PR.

I've updated the CHANGELOG.md as requested.

@viglia viglia requested a review from YangKeao June 9, 2022 08:59
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

LGTM

@viglia
Copy link
Contributor Author

viglia commented Jun 17, 2022

@YangKeao thanks for merging the other PR.

I think this is good to go as well?

@YangKeao YangKeao merged commit ee4022d into tikv:master Jun 20, 2022
Jardynq pushed a commit to Jardynq/pprof-rs that referenced this pull request Jul 24, 2022
* Adds timing info to each sample

Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>

* Updates the CHANGELOG.md

Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>

* Add PR info to updated CHANGELOG.md

Signed-off-by: Francesco Vigliaturo <viglia90@gmail.com>

Co-authored-by: YangKeao <yangkeao@chunibyo.icu>
Signed-off-by: Christian Jordan <svorre2304@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants