Skip to content

Commit d304542

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 <brian.troutwine@datadoghq.com>
1 parent 2a09dca commit d304542

File tree

1 file changed

+56
-61
lines changed

1 file changed

+56
-61
lines changed

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

Lines changed: 56 additions & 61 deletions
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,66 +289,65 @@ 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;
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.size.by_pathname", &labels).set(measures.size as f64);
306+
gauge!("smaps.swap.by_pathname", &labels).set(measures.swap as f64);
307+
gauge!("smaps.private_clean.by_pathname", &labels)
308+
.set(measures.private_clean as f64);
309+
gauge!("smaps.private_dirty.by_pathname", &labels)
310+
.set(measures.private_dirty as f64);
311+
gauge!("smaps.shared_clean.by_pathname", &labels)
312+
.set(measures.shared_clean as f64);
313+
gauge!("smaps.shared_dirty.by_pathname", &labels)
314+
.set(measures.shared_dirty as f64);
315+
gauge!("smaps.referenced.by_pathname", &labels)
316+
.set(measures.referenced as f64);
317+
gauge!("smaps.anonymous.by_pathname", &labels)
318+
.set(measures.anonymous as f64);
319+
gauge!("smaps.lazy_free.by_pathname", &labels)
320+
.set(measures.lazy_free as f64);
321+
gauge!("smaps.anon_huge_pages.by_pathname", &labels)
322+
.set(measures.anon_huge_pages as f64);
323+
gauge!("smaps.shmem_pmd_mapped.by_pathname", &labels)
324+
.set(measures.shmem_pmd_mapped as f64);
325+
gauge!("smaps.shared_hugetlb.by_pathname", &labels)
326+
.set(measures.shared_hugetlb as f64);
327+
gauge!("smaps.private_hugetlb.by_pathname", &labels)
328+
.set(measures.private_hugetlb as f64);
329+
gauge!("smaps.file_pmd_mapped.by_pathname", &labels)
330+
.set(measures.file_pmd_mapped as f64);
331+
gauge!("smaps.locked.by_pathname", &labels).set(measures.locked as f64);
332+
gauge!("smaps.swap_pss.by_pathname", &labels)
333+
.set(measures.swap_pss as f64);
304334
}
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.size.by_pathname", &labels).set(measures.size as f64);
317-
gauge!("smaps.swap.by_pathname", &labels).set(measures.swap as f64);
318-
gauge!("smaps.private_clean.by_pathname", &labels)
319-
.set(measures.private_clean as f64);
320-
gauge!("smaps.private_dirty.by_pathname", &labels)
321-
.set(measures.private_dirty as f64);
322-
gauge!("smaps.shared_clean.by_pathname", &labels)
323-
.set(measures.shared_clean as f64);
324-
gauge!("smaps.shared_dirty.by_pathname", &labels)
325-
.set(measures.shared_dirty as f64);
326-
gauge!("smaps.referenced.by_pathname", &labels)
327-
.set(measures.referenced as f64);
328-
gauge!("smaps.anonymous.by_pathname", &labels)
329-
.set(measures.anonymous as f64);
330-
gauge!("smaps.lazy_free.by_pathname", &labels)
331-
.set(measures.lazy_free as f64);
332-
gauge!("smaps.anon_huge_pages.by_pathname", &labels)
333-
.set(measures.anon_huge_pages as f64);
334-
gauge!("smaps.shmem_pmd_mapped.by_pathname", &labels)
335-
.set(measures.shmem_pmd_mapped as f64);
336-
gauge!("smaps.shared_hugetlb.by_pathname", &labels)
337-
.set(measures.shared_hugetlb as f64);
338-
gauge!("smaps.private_hugetlb.by_pathname", &labels)
339-
.set(measures.private_hugetlb as f64);
340-
gauge!("smaps.file_pmd_mapped.by_pathname", &labels)
341-
.set(measures.file_pmd_mapped as f64);
342-
gauge!("smaps.locked.by_pathname", &labels).set(measures.locked as f64);
343-
gauge!("smaps.swap_pss.by_pathname", &labels).set(measures.swap_pss as f64);
344335
}
345-
346-
// `/proc/{pid}/smaps_rollup`
347-
if let Err(err) = memory::smaps_rollup::poll(pid, &labels).await {
348-
// We don't want to bail out entirely if we can't read smap rollup
336+
Err(err) => {
337+
// We don't want to bail out entirely if we can't read stats
349338
// which will happen if we don't have permissions or, more
350339
// likely, the process has exited.
351-
warn!("Couldn't process `/proc/{pid}/smaps_rollup`: {err}");
340+
warn!("Couldn't process `/proc/{pid}/smaps`: {err}");
352341
}
353-
});
342+
}
343+
344+
// `/proc/{pid}/smaps_rollup`
345+
if let Err(err) = memory::smaps_rollup::poll(pid, &labels).await {
346+
// We don't want to bail out entirely if we can't read smap rollup
347+
// which will happen if we don't have permissions or, more
348+
// likely, the process has exited.
349+
warn!("Couldn't process `/proc/{pid}/smaps_rollup`: {err}");
350+
}
354351
}
355352
}
356353

@@ -418,8 +415,6 @@ impl Sampler {
418415
self.previous_totals = total_sample;
419416
self.previous_samples = samples;
420417

421-
// drain any tasks before returning
422-
while (joinset.join_next().await).is_some() {}
423418
Ok(())
424419
}
425420
}

0 commit comments

Comments
 (0)