From 38fc5430ffbaf372ec2e6256daac72d6f750cb13 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 14 May 2021 10:29:12 +0200 Subject: [PATCH 1/3] Refactor: Be more idomatic with iterator API Signed-off-by: Matthias Beyer --- src/filter.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/filter.rs b/src/filter.rs index 56d4ba3..279b7ae 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -4,12 +4,7 @@ pub fn filter(block_list: &Vec, buf: &[u8]) -> String { let statsd_str = unsafe { str::from_utf8_unchecked(&buf) }; let result_itr = statsd_str.split("\n").filter(|line| { - for prefix in block_list.iter() { - if line.starts_with(prefix) { - return false; - } - } - return true; + !block_list.iter().any(|prefix| line.starts_with(prefix)) }); let result = result_itr.collect::>().join("\n"); From c6c5fb854d30f23df33201ab34c0e1f8717868b3 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 14 May 2021 10:30:27 +0200 Subject: [PATCH 2/3] Refactor: Be more idomatic using iterator API, remove return keyword Signed-off-by: Matthias Beyer --- src/filter.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/filter.rs b/src/filter.rs index 279b7ae..0ff5965 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -3,13 +3,12 @@ use std::str; pub fn filter(block_list: &Vec, buf: &[u8]) -> String { let statsd_str = unsafe { str::from_utf8_unchecked(&buf) }; - let result_itr = statsd_str.split("\n").filter(|line| { - !block_list.iter().any(|prefix| line.starts_with(prefix)) - }); - - let result = result_itr.collect::>().join("\n"); - - return result; + statsd_str.split("\n") + .filter(|line| { + !block_list.iter().any(|prefix| line.starts_with(prefix)) + }) + .collect::>() + .join("\n") } #[cfg(test)] From 22b88ceab2de683ecf152255013781568773f275 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 14 May 2021 10:38:56 +0200 Subject: [PATCH 3/3] Refactor: Only join string parts if there are any This refactoring might have a performance impact (cannot measure). With this patch, the return value in the `filter()` function is not a `String` anymore, but a list of references: `Vec<&str>`. Previously, the `String` was only processed further if it was non-empty. An empty Vec<&str> is the same as an empty `String`: Not relevant for further processing. Hence, we can optimize this and join the `Vec<&str>` to `String` only if it is not empty. Signed-off-by: Matthias Beyer --- src/filter.rs | 13 ++++++------- src/server.rs | 10 ++++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/filter.rs b/src/filter.rs index 0ff5965..7425bd7 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -1,6 +1,6 @@ use std::str; -pub fn filter(block_list: &Vec, buf: &[u8]) -> String { +pub fn filter<'a>(block_list: &'a Vec, buf: &'a [u8]) -> Vec<&'a str> { let statsd_str = unsafe { str::from_utf8_unchecked(&buf) }; statsd_str.split("\n") @@ -8,7 +8,6 @@ pub fn filter(block_list: &Vec, buf: &[u8]) -> String { !block_list.iter().any(|prefix| line.starts_with(prefix)) }) .collect::>() - .join("\n") } #[cfg(test)] @@ -19,7 +18,7 @@ mod tests { fn test_should_not_block_multi_metric() { let block_list = vec![String::from("notfoo"), String::from("otherfoo")]; let statsd_str_bytes = "foo:1|c\nfoo:2|c\nfoo:3|c".as_bytes(); - let result = filter(&block_list, &statsd_str_bytes); + let result = filter(&block_list, &statsd_str_bytes).join("\n"); assert_eq!("foo:1|c\nfoo:2|c\nfoo:3|c", result); } @@ -28,7 +27,7 @@ mod tests { fn test_should_not_block_single_metric() { let block_list = vec![String::from("notfoo"), String::from("otherfoo")]; let statsd_str_bytes = "foo:1|c".as_bytes(); - let result = filter(&block_list, &statsd_str_bytes); + let result = filter(&block_list, &statsd_str_bytes).join("\n"); assert_eq!("foo:1|c", result); } @@ -36,7 +35,7 @@ mod tests { fn test_should_block_completely_single_metric() { let block_list = vec![String::from("foo"), String::from("otherfoo")]; let statsd_str_bytes = "foo:1|c".as_bytes(); - let result = filter(&block_list, &statsd_str_bytes); + let result = filter(&block_list, &statsd_str_bytes).join("\n"); assert_eq!("", result); } @@ -44,7 +43,7 @@ mod tests { fn test_should_block_completely_multi_metric() { let block_list = vec![String::from("foo"), String::from("otherfoo")]; let statsd_str_bytes = "foo:1|c\nfoo:2|c\nfoo:3|c".as_bytes(); - let result = filter(&block_list, &statsd_str_bytes); + let result = filter(&block_list, &statsd_str_bytes).join("\n"); assert_eq!("", result); } @@ -52,7 +51,7 @@ mod tests { fn test_should_block_partially_multi_metric() { let block_list = vec![String::from("foo"), String::from("otherfoo")]; let statsd_str_bytes = "notfoo:1|c\nfoo:2|c\nnotfoo:3|c".as_bytes(); - let result = filter(&block_list, &statsd_str_bytes); + let result = filter(&block_list, &statsd_str_bytes).join("\n"); assert_eq!("notfoo:1|c\nnotfoo:3|c", result); } } diff --git a/src/server.rs b/src/server.rs index c4d276e..4012509 100644 --- a/src/server.rs +++ b/src/server.rs @@ -44,8 +44,9 @@ pub async fn run_server(config: Config) -> io::Result<()> { let target_addr_clone = target_addr.clone(); let blocklist_clone = blocklist.clone(); tokio::spawn(async move { - let filtered_string = filter(&blocklist_clone, &buf[..len]); - if filtered_string.len() > 0 { + let filtered_string_parts = filter(&blocklist_clone, &buf[..len]); + if !filtered_string_parts.is_empty() { + let filtered_string = filtered_string_parts.join("\n"); let len = sock_clone .send_to(filtered_string.as_bytes(), &*target_addr_clone) .await @@ -60,8 +61,9 @@ pub async fn run_server(config: Config) -> io::Result<()> { } }); } else { - let filtered_string = filter(&blocklist, &buf[..len]); - if filtered_string.len() > 0 { + let filtered_string_parts = filter(&blocklist, &buf[..len]); + if !filtered_string_parts.is_empty() { + let filtered_string = filtered_string_parts.join("\n"); let len = sock .send_to(filtered_string.as_bytes(), &*target_addr) .await