From 623091133088555883ee91103e0eda46d4aba939 Mon Sep 17 00:00:00 2001 From: Malisa Smith Date: Thu, 7 Jul 2016 10:18:10 -0700 Subject: [PATCH 1/7] separate profiler statistics calculation to its own function --- components/profile/time.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/components/profile/time.rs b/components/profile/time.rs index 9c9debdb738..f9a70d04b13 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -327,6 +327,18 @@ impl Profiler { true } + + fn get_statistics(data: &[f64]) -> (f64, f64, f64, f64) { + let data_len = data.len(); + let (mean, median, min, max) = + (data.iter().sum::() / (data_len as f64), + data[data_len / 2], + data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), + data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); + (mean, median, min, max) + } + + fn print_buckets(&mut self) { match self.output { Some(OutputOptions::FileName(ref filename)) => { @@ -349,11 +361,7 @@ impl Profiler { }); let data_len = data.len(); if data_len > 0 { - let (mean, median, min, max) = - (data.iter().sum::() / (data_len as f64), - data[data_len / 2], - data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), - data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); + let (mean, median, min, max) = Self::get_statistics(data); write!(file, "{}\t{}\t{:15.4}\t{:15.4}\t{:15.4}\t{:15.4}\t{:15}\n", category.format(&self.output), meta.format(&self.output), mean, median, min, max, data_len).unwrap(); @@ -378,11 +386,7 @@ impl Profiler { }); let data_len = data.len(); if data_len > 0 { - let (mean, median, min, max) = - (data.iter().sum::() / (data_len as f64), - data[data_len / 2], - data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), - data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); + let (mean, median, min, max) = Self::get_statistics(data); writeln!(&mut lock, "{:-35}{} {:15.4} {:15.4} {:15.4} {:15.4} {:15}", category.format(&self.output), meta.format(&self.output), mean, median, min, max, data_len).unwrap(); From d47b769cf7734b20cc36e6b310359e25d43e0b3d Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Thu, 7 Jul 2016 11:00:56 -0700 Subject: [PATCH 2/7] Add test for Profiler::get_statistics, and remove empty space around Profiler::get_statistics. Profiler::get_statistics calculates mean, median, min, and max for a given vector of data. This commit adds a test for the calculated statistics. --- components/profile/time.rs | 4 +--- tests/unit/profile/time.rs | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/components/profile/time.rs b/components/profile/time.rs index f9a70d04b13..4659178b506 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -327,8 +327,7 @@ impl Profiler { true } - - fn get_statistics(data: &[f64]) -> (f64, f64, f64, f64) { + pub fn get_statistics(data: &[f64]) -> (f64, f64, f64, f64) { let data_len = data.len(); let (mean, median, min, max) = (data.iter().sum::() / (data_len as f64), @@ -338,7 +337,6 @@ impl Profiler { (mean, median, min, max) } - fn print_buckets(&mut self) { match self.output { Some(OutputOptions::FileName(ref filename)) => { diff --git a/tests/unit/profile/time.rs b/tests/unit/profile/time.rs index 70daaf2cb73..114959dad23 100644 --- a/tests/unit/profile/time.rs +++ b/tests/unit/profile/time.rs @@ -15,3 +15,24 @@ fn time_profiler_smoke_test() { chan.send(ProfilerMsg::Exit(ipcchan)); assert!(true, "Can tell the profiler thread to exit"); } + +#[test] +fn time_profilers_stats_test() { + let even_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, \ + 9.2345, 10.2342345, 13.2599, 15.0]; + let (even_mean, even_median, even_min, even_max) = time::Profiler::get_statistics(&even_data); + + assert_eq!(7.34230845, even_mean); + assert_eq!(7.345, even_median); + assert_eq!(1.234, even_min); + assert_eq!(15.0, even_max); + + let odd_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, \ + 9.2345, 10.2342345, 13.2599]; + let (odd_mean, odd_median, odd_min, odd_max) = time::Profiler::get_statistics(&odd_data); + + assert_eq!(6.491453833333334, odd_mean); + assert_eq!(5.324, odd_median); + assert_eq!(1.234, odd_min); + assert_eq!(13.2599, odd_max); +} From 7dccf09ff6800c7da2e50bcccc663f39bdec369f Mon Sep 17 00:00:00 2001 From: Malisa Smith Date: Thu, 7 Jul 2016 12:59:16 -0700 Subject: [PATCH 3/7] added test and assertion that profiler statistics data must be sorted --- components/profile/time.rs | 17 +++++++++++------ tests/unit/profile/time.rs | 14 +++++++++++--- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/components/profile/time.rs b/components/profile/time.rs index 4659178b506..76ac26497e9 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -328,12 +328,17 @@ impl Profiler { } pub fn get_statistics(data: &[f64]) -> (f64, f64, f64, f64) { - let data_len = data.len(); - let (mean, median, min, max) = - (data.iter().sum::() / (data_len as f64), - data[data_len / 2], - data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), - data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); + data.iter().fold(-f64::INFINITY, |a, &b| { + debug_assert!(a < b, "Data must be sorted"); + b + }); + + let data_len = data.len(); + let (mean, median, min, max) = + (data.iter().sum::() / (data_len as f64), + data[data_len / 2], + data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), + data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); (mean, median, min, max) } diff --git a/tests/unit/profile/time.rs b/tests/unit/profile/time.rs index 114959dad23..0edbb596de4 100644 --- a/tests/unit/profile/time.rs +++ b/tests/unit/profile/time.rs @@ -17,8 +17,8 @@ fn time_profiler_smoke_test() { } #[test] -fn time_profilers_stats_test() { - let even_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, \ +fn time_profiler_stats_test() { + let even_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, 9.2345, 10.2342345, 13.2599, 15.0]; let (even_mean, even_median, even_min, even_max) = time::Profiler::get_statistics(&even_data); @@ -27,7 +27,7 @@ fn time_profilers_stats_test() { assert_eq!(1.234, even_min); assert_eq!(15.0, even_max); - let odd_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, \ + let odd_data = vec![1.234, 3.24567, 3.54578, 5.0, 5.324, 7.345, 9.2345, 10.2342345, 13.2599]; let (odd_mean, odd_median, odd_min, odd_max) = time::Profiler::get_statistics(&odd_data); @@ -36,3 +36,11 @@ fn time_profilers_stats_test() { assert_eq!(1.234, odd_min); assert_eq!(13.2599, odd_max); } + +#[cfg(debug)] +#[test] +#[should_panic] +fn time_profiler_unsorted_stats_test() { + let unsorted_data = vec![5.0, 7.5, 1.0, 8.9]; + time::Profiler::get_statistics(&unsorted_data); +} From e1092b8b1da4c7c9c778457b0b06c6fa82058078 Mon Sep 17 00:00:00 2001 From: Malisa Smith Date: Thu, 7 Jul 2016 13:08:20 -0700 Subject: [PATCH 4/7] simplifying min and max calculation for profiler get_statistics function --- components/profile/time.rs | 5 +++-- tests/unit/profile/time.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/components/profile/time.rs b/components/profile/time.rs index 76ac26497e9..ca9d8070d9f 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -334,11 +334,12 @@ impl Profiler { }); let data_len = data.len(); + debug_assert!(data_len > 0); let (mean, median, min, max) = (data.iter().sum::() / (data_len as f64), data[data_len / 2], - data.iter().fold(f64::INFINITY, |a, &b| a.min(b)), - data.iter().fold(-f64::INFINITY, |a, &b| a.max(b))); + data[0], + data[data_len-1]); (mean, median, min, max) } diff --git a/tests/unit/profile/time.rs b/tests/unit/profile/time.rs index 0edbb596de4..9bae5801262 100644 --- a/tests/unit/profile/time.rs +++ b/tests/unit/profile/time.rs @@ -37,7 +37,7 @@ fn time_profiler_stats_test() { assert_eq!(13.2599, odd_max); } -#[cfg(debug)] +#[cfg(debug_assertions)] #[test] #[should_panic] fn time_profiler_unsorted_stats_test() { From b7510bd89f54f0ccb9040f41d911be2aa30f46f0 Mon Sep 17 00:00:00 2001 From: Jeena Lee Date: Thu, 7 Jul 2016 13:14:35 -0700 Subject: [PATCH 5/7] Add test to assert that servo panics when the length of a given data is zero. --- tests/unit/profile/time.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/profile/time.rs b/tests/unit/profile/time.rs index 9bae5801262..911f1cf95c0 100644 --- a/tests/unit/profile/time.rs +++ b/tests/unit/profile/time.rs @@ -44,3 +44,11 @@ fn time_profiler_unsorted_stats_test() { let unsorted_data = vec![5.0, 7.5, 1.0, 8.9]; time::Profiler::get_statistics(&unsorted_data); } + +#[cfg(debug_assertions)] +#[test] +#[should_panic] +fn time_profiler_data_len_zero() { + let zero_data = vec![]; + time::Profiler::get_statistics(&zero_data); +} From 854cb5839fbf8fa5a4d9d58ecdf260f7740faff0 Mon Sep 17 00:00:00 2001 From: Malisa Smith Date: Thu, 7 Jul 2016 13:19:45 -0700 Subject: [PATCH 6/7] added comment to profiler get_statistics function --- components/profile/time.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/profile/time.rs b/components/profile/time.rs index ca9d8070d9f..39589536eac 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -327,6 +327,7 @@ impl Profiler { true } + /// Get tuple (mean, median, min, max) for profiler statistics. pub fn get_statistics(data: &[f64]) -> (f64, f64, f64, f64) { data.iter().fold(-f64::INFINITY, |a, &b| { debug_assert!(a < b, "Data must be sorted"); From 1a1d8daede5eb0f6c59e61eb1f83416bbc723ecc Mon Sep 17 00:00:00 2001 From: Malisa Smith Date: Thu, 7 Jul 2016 13:29:58 -0700 Subject: [PATCH 7/7] added spaces around - sign --- components/profile/time.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/profile/time.rs b/components/profile/time.rs index 39589536eac..e6cb6d45f59 100644 --- a/components/profile/time.rs +++ b/components/profile/time.rs @@ -340,7 +340,7 @@ impl Profiler { (data.iter().sum::() / (data_len as f64), data[data_len / 2], data[0], - data[data_len-1]); + data[data_len - 1]); (mean, median, min, max) }