From 0a5a4114349e45e6fb93cf5a08465cbe47067be3 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Fri, 28 May 2021 22:40:03 -0700 Subject: [PATCH 1/6] perf improvments - resuse the regex to parse the output of libfuzzer - added a cancellation notification to report_fuzzer_sys_info. The code seems to be actively waiting this function and consuming some cpu time --- .../src/tasks/fuzz/libfuzzer_fuzz.rs | 44 +++++++++++++------ src/agent/onefuzz/src/libfuzzer.rs | 37 ++++++++++------ 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs b/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs index 602f46904d..789e35b653 100644 --- a/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs +++ b/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs @@ -7,7 +7,7 @@ use arraydeque::{ArrayDeque, Wrapping}; use futures::future::try_join_all; use onefuzz::{ fs::list_files, - libfuzzer::{LibFuzzer, LibFuzzerLine}, + libfuzzer::{LibFuzzer, LibFuzzerLineParser}, process::ExitStatus, syncdir::{continuous_sync, SyncOperation::Pull, SyncedDir}, system, @@ -17,11 +17,12 @@ use onefuzz_telemetry::{ EventData, }; use serde::Deserialize; -use std::{collections::HashMap, path::PathBuf}; +use std::{collections::HashMap, path::PathBuf, sync::Arc}; use tempfile::{tempdir_in, TempDir}; use tokio::{ io::{AsyncBufReadExt, BufReader}, - sync::mpsc, + select, + sync::{mpsc, Notify}, task, time::{sleep, Duration}, }; @@ -212,11 +213,12 @@ impl LibFuzzerFuzzTask { ); let mut running = fuzzer.fuzz(crash_dir.path(), local_inputs, &inputs)?; let running_id = running.id(); - + let notify = Arc::new(Notify::new()); let sys_info = task::spawn(report_fuzzer_sys_info( worker_id, run_id, running_id.unwrap_or_default(), + notify.clone(), )); // Splitting borrow. @@ -227,7 +229,7 @@ impl LibFuzzerFuzzTask { let mut stderr = BufReader::new(stderr); let mut libfuzzer_output: ArrayDeque<[_; LOGS_BUFFER_SIZE], Wrapping> = ArrayDeque::new(); - + let parser = LibFuzzerLineParser::new()?; loop { let mut buf = vec![]; let bytes_read = stderr.read_until(b'\n', &mut buf).await?; @@ -236,14 +238,19 @@ impl LibFuzzerFuzzTask { } let line = String::from_utf8_lossy(&buf).to_string(); if let Some(stats_sender) = stats_sender { - if let Err(err) = try_report_iter_update(stats_sender, worker_id, run_id, &line) { + if let Err(err) = + try_report_iter_update(stats_sender, worker_id, run_id, &line, &parser) + { error!("could not parse fuzzing interation update: {}", err); } } libfuzzer_output.push_back(line); } - let (exit_status, _) = tokio::join!(running.wait(), sys_info); + let exit_status = running.wait().await; + notify.notify_one(); + let _ = sys_info.await; + let exit_status: ExitStatus = exit_status?.into(); let files = list_files(crash_dir.path()).await?; @@ -310,8 +317,9 @@ fn try_report_iter_update( worker_id: usize, run_id: Uuid, line: &str, + parser: &LibFuzzerLineParser, ) -> Result<()> { - if let Some(line) = LibFuzzerLine::parse(line)? { + if let Some(line) = parser.parse(line)? { stats_sender.send(RuntimeStats { worker_id, run_id, @@ -323,11 +331,23 @@ fn try_report_iter_update( Ok(()) } -async fn report_fuzzer_sys_info(worker_id: usize, run_id: Uuid, fuzzer_pid: u32) -> Result<()> { +async fn report_fuzzer_sys_info( + worker_id: usize, + run_id: Uuid, + fuzzer_pid: u32, + cancellation: Arc, +) -> Result<()> { // Allow for sampling CPU usage. - sleep(PROC_INFO_COLLECTION_DELAY).await; - + let mut period = tokio::time::interval_at( + tokio::time::Instant::now() + PROC_INFO_COLLECTION_DELAY, + PROC_INFO_PERIOD, + ); loop { + select! { + () = cancellation.notified() => break, + _ = period.tick() => (), + } + // process doesn't exist if !system::refresh_process(fuzzer_pid)? { break; @@ -348,8 +368,6 @@ async fn report_fuzzer_sys_info(worker_id: usize, run_id: Uuid, fuzzer_pid: u32) // The process no longer exists. break; } - - sleep(PROC_INFO_PERIOD).await; } Ok(()) diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index 42e3fe6a6e..813899f538 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -288,6 +288,29 @@ impl<'a> LibFuzzer<'a> { } } +pub struct LibFuzzerLineParser { + regex: regex::Regex, +} + +impl LibFuzzerLineParser { + pub fn new() -> Result { + let regex = regex::Regex::new(r"#(\d+)\s*(?:pulse|INITED|NEW|REDUCE).*exec/s: (\d+)")?; + Ok(Self { regex }) + } + + pub fn parse(&self, line: &str) -> Result> { + let caps = match self.regex.captures(line) { + Some(caps) => caps, + None => return Ok(None), + }; + + let iters = caps[1].parse()?; + let execs_sec = caps[2].parse()?; + + Ok(Some(LibFuzzerLine::new(line.to_string(), iters, execs_sec))) + } +} + pub struct LibFuzzerLine { _line: String, iters: u64, @@ -303,20 +326,6 @@ impl LibFuzzerLine { } } - pub fn parse(line: &str) -> Result> { - let re = regex::Regex::new(r"#(\d+)\s*(?:pulse|INITED|NEW|REDUCE).*exec/s: (\d+)")?; - - let caps = match re.captures(line) { - Some(caps) => caps, - None => return Ok(None), - }; - - let iters = caps[1].parse()?; - let execs_sec = caps[2].parse()?; - - Ok(Some(Self::new(line.to_string(), iters, execs_sec))) - } - pub fn iters(&self) -> u64 { self.iters } From adb67c1112e88a1c1b045f07f9dc97d52915d947 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Sat, 29 May 2021 19:24:47 -0700 Subject: [PATCH 2/6] fix test --- src/agent/onefuzz/src/libfuzzer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index 813899f538..58cadff5b1 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -342,8 +342,10 @@ mod tests { #[test] fn test_libfuzzer_line_pulse() { let line = r"#2097152 pulse cov: 11 ft: 11 corp: 6/21b lim: 4096 exec/s: 699050 rss: 562Mb"; + let parser = LibFuzzerLineParser::new().unwrap(); - let parsed = LibFuzzerLine::parse(line) + let parsed = parser + .parse(line) .expect("parse error") .expect("no captures"); From 65007f35ef1c8eed91128c3944682269c94f9949 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 31 May 2021 10:33:00 -0700 Subject: [PATCH 3/6] fix cargo audit --- src/agent/Cargo.lock | 18 +++++++++--------- src/agent/onefuzz/Cargo.toml | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index d1b39b7998..fb91ec9327 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -311,7 +311,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06ed27e177f16d65f0f0c22a213e17c696ace5dd64b14258b52f9417ccb52db4" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils 0.8.3", + "crossbeam-utils 0.8.5", ] [[package]] @@ -322,17 +322,17 @@ checksum = "94af6efb46fef72616855b036a624cf27ba656ffc9be1b9a3c931cfc7749a9a9" dependencies = [ "cfg-if 1.0.0", "crossbeam-epoch", - "crossbeam-utils 0.8.3", + "crossbeam-utils 0.8.5", ] [[package]] name = "crossbeam-epoch" -version = "0.9.3" +version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2584f639eb95fea8c798496315b297cf81b9b58b6d30ab066a75455333cf4b12" +checksum = "4ec02e091aa634e2c3ada4a392989e7c3116673ef0ac5b72232439094d73b7fd" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils 0.8.3", + "crossbeam-utils 0.8.5", "lazy_static", "memoffset", "scopeguard", @@ -351,11 +351,10 @@ dependencies = [ [[package]] name = "crossbeam-utils" -version = "0.8.3" +version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7e9d99fa91428effe99c5c6d4634cdeba32b8cf784fc428a2a687f61a952c49" +checksum = "d82cfc11ce7f2c3faef78d8a684447b40d503d9681acebed6cb728d45940c4db" dependencies = [ - "autocfg", "cfg-if 1.0.0", "lazy_static", ] @@ -1474,6 +1473,7 @@ dependencies = [ "base64", "bytes 1.0.1", "cpp_demangle", + "crossbeam-epoch", "debugger", "dunce", "futures", @@ -2028,7 +2028,7 @@ checksum = "9ab346ac5921dc62ffa9f89b7a773907511cdfa5490c572ae9be1be33e8afa4a" dependencies = [ "crossbeam-channel 0.5.1", "crossbeam-deque", - "crossbeam-utils 0.8.3", + "crossbeam-utils 0.8.5", "lazy_static", "num_cpus", ] diff --git a/src/agent/onefuzz/Cargo.toml b/src/agent/onefuzz/Cargo.toml index a1c8eb0eeb..25c8c1b727 100644 --- a/src/agent/onefuzz/Cargo.toml +++ b/src/agent/onefuzz/Cargo.toml @@ -12,6 +12,7 @@ appinsights = "0.1" async-trait = "0.1" base64 = "0.13" bytes = "1.0.1" +crossbeam-epoch = "0.9.5" dunce = "1.0" futures = "0.3.14" futures-util = "0.3.14" From b72dc47d61851dddb84cd74eb276dddb727d1f15 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 1 Jun 2021 17:10:20 -0700 Subject: [PATCH 4/6] static regex for libfuzzer line parser --- .../src/tasks/fuzz/libfuzzer_fuzz.rs | 10 ++--- src/agent/onefuzz/src/libfuzzer.rs | 38 ++++++++----------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs b/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs index 789e35b653..f9ad0dfd41 100644 --- a/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs +++ b/src/agent/onefuzz-agent/src/tasks/fuzz/libfuzzer_fuzz.rs @@ -7,7 +7,7 @@ use arraydeque::{ArrayDeque, Wrapping}; use futures::future::try_join_all; use onefuzz::{ fs::list_files, - libfuzzer::{LibFuzzer, LibFuzzerLineParser}, + libfuzzer::{LibFuzzer, LibFuzzerLine}, process::ExitStatus, syncdir::{continuous_sync, SyncOperation::Pull, SyncedDir}, system, @@ -229,7 +229,6 @@ impl LibFuzzerFuzzTask { let mut stderr = BufReader::new(stderr); let mut libfuzzer_output: ArrayDeque<[_; LOGS_BUFFER_SIZE], Wrapping> = ArrayDeque::new(); - let parser = LibFuzzerLineParser::new()?; loop { let mut buf = vec![]; let bytes_read = stderr.read_until(b'\n', &mut buf).await?; @@ -238,9 +237,7 @@ impl LibFuzzerFuzzTask { } let line = String::from_utf8_lossy(&buf).to_string(); if let Some(stats_sender) = stats_sender { - if let Err(err) = - try_report_iter_update(stats_sender, worker_id, run_id, &line, &parser) - { + if let Err(err) = try_report_iter_update(stats_sender, worker_id, run_id, &line) { error!("could not parse fuzzing interation update: {}", err); } } @@ -317,9 +314,8 @@ fn try_report_iter_update( worker_id: usize, run_id: Uuid, line: &str, - parser: &LibFuzzerLineParser, ) -> Result<()> { - if let Some(line) = parser.parse(line)? { + if let Some(line) = LibFuzzerLine::parse(line)? { stats_sender.send(RuntimeStats { worker_id, run_id, diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index 58cadff5b1..870de394a6 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -21,6 +21,13 @@ use tokio::process::{Child, Command}; const DEFAULT_MAX_TOTAL_SECONDS: i32 = 10 * 60; +use lazy_static::lazy_static; + +lazy_static! { + static ref LIBFUZZERLINEREGEX: regex::Regex = + regex::Regex::new(r"#(\d+)\s*(?:pulse|INITED|NEW|REDUCE).*exec/s: (\d+)").unwrap(); +} + #[derive(Debug)] pub struct LibFuzzerMergeOutput { pub added_files_count: i32, @@ -288,18 +295,15 @@ impl<'a> LibFuzzer<'a> { } } -pub struct LibFuzzerLineParser { - regex: regex::Regex, +pub struct LibFuzzerLine { + _line: String, + iters: u64, + execs_sec: f64, } -impl LibFuzzerLineParser { - pub fn new() -> Result { - let regex = regex::Regex::new(r"#(\d+)\s*(?:pulse|INITED|NEW|REDUCE).*exec/s: (\d+)")?; - Ok(Self { regex }) - } - - pub fn parse(&self, line: &str) -> Result> { - let caps = match self.regex.captures(line) { +impl LibFuzzerLine { + pub fn parse(line: &str) -> Result> { + let caps = match LIBFUZZERLINEREGEX.captures(line) { Some(caps) => caps, None => return Ok(None), }; @@ -307,17 +311,9 @@ impl LibFuzzerLineParser { let iters = caps[1].parse()?; let execs_sec = caps[2].parse()?; - Ok(Some(LibFuzzerLine::new(line.to_string(), iters, execs_sec))) + Ok(Some(Self::new(line.to_string(), iters, execs_sec))) } -} -pub struct LibFuzzerLine { - _line: String, - iters: u64, - execs_sec: f64, -} - -impl LibFuzzerLine { pub fn new(line: String, iters: u64, execs_sec: f64) -> Self { Self { iters, @@ -342,10 +338,8 @@ mod tests { #[test] fn test_libfuzzer_line_pulse() { let line = r"#2097152 pulse cov: 11 ft: 11 corp: 6/21b lim: 4096 exec/s: 699050 rss: 562Mb"; - let parser = LibFuzzerLineParser::new().unwrap(); - let parsed = parser - .parse(line) + let parsed = LibFuzzerLine::parse(line) .expect("parse error") .expect("no captures"); From 1e011acb76cafb236960f3dfae6f298c389178a2 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 1 Jun 2021 17:43:50 -0700 Subject: [PATCH 5/6] formating --- src/agent/onefuzz/src/libfuzzer.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs index 870de394a6..80fb781b37 100644 --- a/src/agent/onefuzz/src/libfuzzer.rs +++ b/src/agent/onefuzz/src/libfuzzer.rs @@ -302,6 +302,14 @@ pub struct LibFuzzerLine { } impl LibFuzzerLine { + pub fn new(line: String, iters: u64, execs_sec: f64) -> Self { + Self { + iters, + _line: line, + execs_sec, + } + } + pub fn parse(line: &str) -> Result> { let caps = match LIBFUZZERLINEREGEX.captures(line) { Some(caps) => caps, @@ -314,14 +322,6 @@ impl LibFuzzerLine { Ok(Some(Self::new(line.to_string(), iters, execs_sec))) } - pub fn new(line: String, iters: u64, execs_sec: f64) -> Self { - Self { - iters, - _line: line, - execs_sec, - } - } - pub fn iters(&self) -> u64 { self.iters } From c698075447d72506940ca761b895fe41ebe6978c Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 1 Jun 2021 17:49:24 -0700 Subject: [PATCH 6/6] reverting toml file --- src/agent/Cargo.lock | 1 - src/agent/onefuzz/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock index 227d9e83c3..8a0e00a6dc 100644 --- a/src/agent/Cargo.lock +++ b/src/agent/Cargo.lock @@ -1475,7 +1475,6 @@ dependencies = [ "base64", "bytes 1.0.1", "cpp_demangle", - "crossbeam-epoch", "debugger", "dunce", "futures", diff --git a/src/agent/onefuzz/Cargo.toml b/src/agent/onefuzz/Cargo.toml index 25c8c1b727..a1c8eb0eeb 100644 --- a/src/agent/onefuzz/Cargo.toml +++ b/src/agent/onefuzz/Cargo.toml @@ -12,7 +12,6 @@ appinsights = "0.1" async-trait = "0.1" base64 = "0.13" bytes = "1.0.1" -crossbeam-epoch = "0.9.5" dunce = "1.0" futures = "0.3.14" futures-util = "0.3.14"