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

Track both jobs and bytes in each IO state #1507

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Oct 15, 2024

It would be helpful to track how many bytes are associated with each job state, so that we can inject Barrier operations if we're buffering too many Done bytes. This PR adds a new counter for that purpose!

  • Add a io_state_byte_count: ClientIOStateCount<u64> to track number of bytes associated with each IO state
    • Refactors ClientIOStateCount to be generic, instead of u32-specific
    • Change access to ClientIOStateCount to use IndexMut
    • Rename io_state_count to io_state_job_count in DownstairsClient
  • Drive-by: make DownstairsIO::io_size() just call into IOop::job_bytes(), which does the same thing

@@ -2252,7 +2279,8 @@ impl DownstairsClient {
}

pub(crate) fn total_live_work(&self) -> usize {
(self.io_state_count.new + self.io_state_count.in_progress) as usize
(self.io_state_job_count.new + self.io_state_job_count.in_progress)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how this snuck through, but the io_state_job_count.new has no IOState equivalent any longer. Since your in the neighborhood can you just remove the new field from ClientIOStateCount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, removed in 663ae0d

This will change the DTrace probe format, but it looks like no one is using ok.ds_io_count.new

pub error: u32,
#[derive(Debug, Default, Copy, Clone, Serialize, Deserialize)]
pub struct ClientIOStateCount<T = u32> {
pub new: T,
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the new here is now vestigial and we can remove it.

@mkeeter mkeeter force-pushed the mkeeter/io-state-job-and-byte-count branch from 7b7ec22 to 1e4cc53 Compare October 16, 2024 13:48
@mkeeter mkeeter merged commit 0875da3 into main Oct 16, 2024
16 checks passed
@mkeeter mkeeter deleted the mkeeter/io-state-job-and-byte-count branch October 16, 2024 14:20
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.

2 participants