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

feat(#305)!: Expose new PiscinaHistogram abstraction #723

Merged
merged 4 commits into from
Jan 10, 2025
Merged

Conversation

metcoder95
Copy link
Member

@metcoder95 metcoder95 commented Jan 5, 2025

Closes #305

@metcoder95 metcoder95 requested review from ronag and RafaelGSS January 6, 2025 08:48
ronag
ronag previously approved these changes Jan 6, 2025
@ronag
Copy link
Collaborator

ronag commented Jan 6, 2025

Should we automatically reset the histogram at some interval?

@@ -110,6 +110,53 @@ Use caution when setting resource limits. Setting limits that are too low may
result in the `Piscina` worker threads being unusable.
:::

## `PiscinaHistogram`

The `PiscinaHistogram` allows you to access the histogram data for the pool of worker threads.
Copy link
Member

Choose a reason for hiding this comment

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

Can we have a code sample on how users would do it in userland?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, let me know if that's enough or if there's a something else to cover

@RafaelGSS
Copy link
Member

Should we automatically reset the histogram at some interval?

I don't think so, since we are storing distribution data (mean, avg, stddev)

@metcoder95 metcoder95 requested a review from RafaelGSS January 9, 2025 07:37
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM.

I think the PR title doesn't express entirely what this PR does. It also exposes the PiscinaHistogram

@metcoder95 metcoder95 changed the title feat(#305)!: allow to reset histograms feat(#305)!: Expose new PiscinaHistogram abstraction Jan 9, 2025
@metcoder95 metcoder95 merged commit 86d736c into current Jan 10, 2025
11 checks passed
@metcoder95 metcoder95 deleted the feat/305 branch January 10, 2025 09:13
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.

feat: allow to reset histograms
3 participants