Skip to content

Commit 88a426e

Browse files
bltblt
committed
Slim out joinset
This commit removes a joinset in the inner polling loop of the procfs sampler. The existence of the joinset makes calculation of total_pss, total_rss more difficult and its unclear that there is a performance boost by its existence. Signed-off-by: Brian L. Troutwine <[email protected]>
1 parent d767aa4 commit 88a426e

File tree

1 file changed

+72
-78
lines changed

1 file changed

+72
-78
lines changed

‎lading/src/observer/linux/procfs.rs

+72-78
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,12 @@ impl Sampler {
8585
clippy::cast_possible_wrap
8686
)]
8787
pub(crate) async fn poll(&mut self) -> Result<(), Error> {
88-
let mut joinset = tokio::task::JoinSet::new();
8988
// Key for this map is (pid, basename/exe, cmdline)
9089
let mut samples: FxHashMap<ProcessIdentifier, Sample> = FxHashMap::default();
9190

9291
let mut total_processes: u64 = 0;
93-
// We maintain a tally of the total RSS consumed by the parent process
94-
// and its children. This is used for analysis and for cooperation with
95-
// the throttle (through RSS_BYTES).
92+
// We maintain a tally of the total RSS and PSS consumed by the parent
93+
// process and its children.
9694
let mut total_rss: u64 = 0;
9795

9896
// Calculate the ticks since machine uptime. This will be important
@@ -291,83 +289,81 @@ impl Sampler {
291289
// If a previous call to process.status() failed due to a lack of ptrace permissions,
292290
// then we assume smaps operations will fail as well.
293291
if has_ptrace_perm {
294-
joinset.spawn(async move {
295-
// `/proc/{pid}/smaps`
296-
let memory_regions = match memory::smaps::Regions::from_pid(pid) {
297-
Ok(memory_regions) => memory_regions,
298-
Err(e) => {
299-
// We don't want to bail out entirely if we can't read stats
300-
// which will happen if we don't have permissions or, more
301-
// likely, the process has exited.
302-
warn!("Couldn't process `/proc/{pid}/smaps`: {}", e);
303-
return;
304-
}
305-
};
306-
for (pathname, measures) in memory_regions.aggregate_by_pathname() {
307-
let labels = [
308-
("pid", format!("{pid}")),
309-
("exe", basename.clone()),
310-
("cmdline", cmdline.clone()),
311-
("comm", comm.clone()),
312-
("pathname", pathname),
313-
];
314-
gauge!("smaps.rss.by_pathname", &labels).set(measures.rss as f64);
315-
gauge!("smaps.pss.by_pathname", &labels).set(measures.pss as f64);
316-
gauge!("smaps.swap.by_pathname", &labels).set(measures.swap as f64);
317-
gauge!("smaps.size.by_pathname", &labels).set(measures.size as f64);
318-
319-
if let Some(m) = measures.private_clean {
320-
gauge!("smaps.private_clean.by_pathname", &labels).set(m as f64);
321-
}
322-
if let Some(m) = measures.private_dirty {
323-
gauge!("smaps.private_dirty.by_pathname", &labels).set(m as f64);
324-
}
325-
if let Some(m) = measures.shared_clean {
326-
gauge!("smaps.shared_clean.by_pathname", &labels).set(m as f64);
327-
}
328-
if let Some(m) = measures.shared_dirty {
329-
gauge!("smaps.shared_dirty.by_pathname", &labels).set(m as f64);
330-
}
331-
if let Some(m) = measures.referenced {
332-
gauge!("smaps.referenced.by_pathname", &labels).set(m as f64);
333-
}
334-
if let Some(m) = measures.anonymous {
335-
gauge!("smaps.anonymous.by_pathname", &labels).set(m as f64);
336-
}
337-
if let Some(m) = measures.lazy_free {
338-
gauge!("smaps.lazy_free.by_pathname", &labels).set(m as f64);
339-
}
340-
if let Some(m) = measures.anon_huge_pages {
341-
gauge!("smaps.anon_huge_pages.by_pathname", &labels).set(m as f64);
342-
}
343-
if let Some(m) = measures.shmem_pmd_mapped {
344-
gauge!("smaps.shmem_pmd_mapped.by_pathname", &labels).set(m as f64);
345-
}
346-
if let Some(m) = measures.shared_hugetlb {
347-
gauge!("smaps.shared_hugetlb.by_pathname", &labels).set(m as f64);
348-
}
349-
if let Some(m) = measures.private_hugetlb {
350-
gauge!("smaps.private_hugetlb.by_pathname", &labels).set(m as f64);
351-
}
352-
if let Some(m) = measures.file_pmd_mapped {
353-
gauge!("smaps.file_pmd_mapped.by_pathname", &labels).set(m as f64);
354-
}
355-
if let Some(m) = measures.locked {
356-
gauge!("smaps.locked.by_pathname", &labels).set(m as f64);
357-
}
358-
if let Some(m) = measures.swap_pss {
359-
gauge!("smaps.swap_pss.by_pathname", &labels).set(m as f64);
292+
// `/proc/{pid}/smaps`
293+
match memory::smaps::Regions::from_pid(pid) {
294+
Ok(memory_regions) => {
295+
for (pathname, measures) in memory_regions.aggregate_by_pathname() {
296+
let labels = [
297+
("pid", format!("{pid}")),
298+
("exe", basename.clone()),
299+
("cmdline", cmdline.clone()),
300+
("comm", comm.clone()),
301+
("pathname", pathname),
302+
];
303+
gauge!("smaps.rss.by_pathname", &labels).set(measures.rss as f64);
304+
gauge!("smaps.pss.by_pathname", &labels).set(measures.pss as f64);
305+
gauge!("smaps.swap.by_pathname", &labels).set(measures.swap as f64);
306+
gauge!("smaps.size.by_pathname", &labels).set(measures.size as f64);
307+
308+
if let Some(m) = measures.private_clean {
309+
gauge!("smaps.private_clean.by_pathname", &labels).set(m as f64);
310+
}
311+
if let Some(m) = measures.private_dirty {
312+
gauge!("smaps.private_dirty.by_pathname", &labels).set(m as f64);
313+
}
314+
if let Some(m) = measures.shared_clean {
315+
gauge!("smaps.shared_clean.by_pathname", &labels).set(m as f64);
316+
}
317+
if let Some(m) = measures.shared_dirty {
318+
gauge!("smaps.shared_dirty.by_pathname", &labels).set(m as f64);
319+
}
320+
if let Some(m) = measures.referenced {
321+
gauge!("smaps.referenced.by_pathname", &labels).set(m as f64);
322+
}
323+
if let Some(m) = measures.anonymous {
324+
gauge!("smaps.anonymous.by_pathname", &labels).set(m as f64);
325+
}
326+
if let Some(m) = measures.lazy_free {
327+
gauge!("smaps.lazy_free.by_pathname", &labels).set(m as f64);
328+
}
329+
if let Some(m) = measures.anon_huge_pages {
330+
gauge!("smaps.anon_huge_pages.by_pathname", &labels).set(m as f64);
331+
}
332+
if let Some(m) = measures.shmem_pmd_mapped {
333+
gauge!("smaps.shmem_pmd_mapped.by_pathname", &labels).set(m as f64);
334+
}
335+
if let Some(m) = measures.shared_hugetlb {
336+
gauge!("smaps.shared_hugetlb.by_pathname", &labels).set(m as f64);
337+
}
338+
if let Some(m) = measures.private_hugetlb {
339+
gauge!("smaps.private_hugetlb.by_pathname", &labels).set(m as f64);
340+
}
341+
if let Some(m) = measures.file_pmd_mapped {
342+
gauge!("smaps.file_pmd_mapped.by_pathname", &labels).set(m as f64);
343+
}
344+
if let Some(m) = measures.locked {
345+
gauge!("smaps.locked.by_pathname", &labels).set(m as f64);
346+
}
347+
if let Some(m) = measures.swap_pss {
348+
gauge!("smaps.swap_pss.by_pathname", &labels).set(m as f64);
349+
}
360350
}
361351
}
362-
363-
// `/proc/{pid}/smaps_rollup`
364-
if let Err(err) = memory::smaps_rollup::poll(pid, &labels).await {
365-
// We don't want to bail out entirely if we can't read smap rollup
352+
Err(err) => {
353+
// We don't want to bail out entirely if we can't read stats
366354
// which will happen if we don't have permissions or, more
367355
// likely, the process has exited.
368-
warn!("Couldn't process `/proc/{pid}/smaps_rollup`: {err}");
356+
warn!("Couldn't process `/proc/{pid}/smaps`: {err}");
369357
}
370-
});
358+
}
359+
360+
// `/proc/{pid}/smaps_rollup`
361+
if let Err(err) = memory::smaps_rollup::poll(pid, &labels).await {
362+
// We don't want to bail out entirely if we can't read smap rollup
363+
// which will happen if we don't have permissions or, more
364+
// likely, the process has exited.
365+
warn!("Couldn't process `/proc/{pid}/smaps_rollup`: {err}");
366+
}
371367
}
372368
}
373369

@@ -435,8 +431,6 @@ impl Sampler {
435431
self.previous_totals = total_sample;
436432
self.previous_samples = samples;
437433

438-
// drain any tasks before returning
439-
while (joinset.join_next().await).is_some() {}
440434
Ok(())
441435
}
442436
}

0 commit comments

Comments
 (0)