Skip to content

Add method for per-CPU ring draining#294

Open
utpilla wants to merge 2 commits into
microsoft:mainfrom
utpilla:utpilla/Add-per-CPU-ring-draining
Open

Add method for per-CPU ring draining#294
utpilla wants to merge 2 commits into
microsoft:mainfrom
utpilla:utpilla/Add-per-CPU-ring-draining

Conversation

@utpilla

@utpilla utpilla commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Changes

Adds parse_for_cpu(cpu) (Option 1 from #254) - drains and dispatches one CPU's ring, firing event callbacks like parse_all but scoped to a single CPU. This is the final piece of the wakeup-watermark work, completing with_wakeup_watermark() (#276) and perf_fds() (#279).

Motivation

A multi-CPU PerfSession registers one perf fd per CPU with the consumer's event loop (epoll / AsyncFd). The readiness signal is per-CPU, but parse_all drains every ring - so a wakeup on one CPU pulled events from CPUs whose watermark hadn't tripped. parse_for_cpu matches the drain to the signal: one fd fires, one ring drains.

Comment thread one_collect/src/perf_event/rb/source.rs Outdated
hard_page_faults_builder: Option<RingBufBuilder<PageFaults>>,
next_time: Option<u64>,
oldest_cpu: Option<usize>,
cpu_reader_index: HashMap<u32, usize>,

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.

Since we map index to CPU, lets just use a Vec here that expands to the amount of index's we have. This will speed up the lookup compared to a hash.

self.enabled
}

fn begin_reading_cpu(

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.

It would be nice to have begin_reading_cpu() return a struct that contains the reader, cursor that then is passed to read_cpu(). This would eliminate several branches and memory lookups within read_cpu() that is called in a tight loop when on a per-CPU looping path.

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.

It could also contain a copy of the ancillary as well.

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