From ca620992a631397d58aeadb24fed3a4a05da6b8a Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Tue, 27 Aug 2013 21:08:41 -0400 Subject: [PATCH 1/6] minor refactor of profiler --- src/components/util/time.rs | 63 ++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index 4dda2457ef9..381aa6c1ba7 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -42,11 +42,22 @@ pub enum ProfilerCategory { RenderingPrepBuffCategory, RenderingCategory, // hackish but helps prevent errors when adding new categories - NUM_BUCKETS, + NumBuckets, } -// FIXME(#5873) this should be initialized by a NUM_BUCKETS cast, -static BUCKETS: uint = 13; -type ProfilerBuckets = [(ProfilerCategory, ~[float]), ..BUCKETS]; +struct ProfilerBucket { + category: ProfilerCategory, + data: ~[float], +} +impl ProfilerBucket { + fn new(category: ProfilerCategory) -> ProfilerBucket { + ProfilerBucket { + category: category, + data: ~[], + } + } +} +// FIXME(rust#5873) this should be initialized by a NumBuckets cast, +type ProfilerBuckets = [ProfilerBucket, ..13]; pub enum ProfilerMsg { // Normal message used for reporting time @@ -65,26 +76,26 @@ pub struct Profiler { impl ProfilerCategory { // convenience function to not have to cast every time pub fn num_buckets() -> uint { - NUM_BUCKETS as uint + NumBuckets as uint } // enumeration of all ProfilerCategory types // TODO(tkuehn): is there a better way to ensure proper order of categories? fn empty_buckets() -> ProfilerBuckets { let buckets = [ - (CompositingCategory, ~[]), - (LayoutQueryCategory, ~[]), - (LayoutPerformCategory, ~[]), - (LayoutAuxInitCategory, ~[]), - (LayoutSelectorMatchCategory, ~[]), - (LayoutTreeBuilderCategory, ~[]), - (LayoutMainCategory, ~[]), - (LayoutShapingCategory, ~[]), - (LayoutDispListBuildCategory, ~[]), - (GfxRegenAvailableFontsCategory, ~[]), - (RenderingDrawingCategory, ~[]), - (RenderingPrepBuffCategory, ~[]), - (RenderingCategory, ~[]), + ProfilerBucket::new(CompositingCategory), + ProfilerBucket::new(LayoutQueryCategory), + ProfilerBucket::new(LayoutPerformCategory), + ProfilerBucket::new(LayoutAuxInitCategory), + ProfilerBucket::new(LayoutSelectorMatchCategory), + ProfilerBucket::new(LayoutTreeBuilderCategory), + ProfilerBucket::new(LayoutMainCategory), + ProfilerBucket::new(LayoutShapingCategory), + ProfilerBucket::new(LayoutDispListBuildCategory), + ProfilerBucket::new(GfxRegenAvailableFontsCategory), + ProfilerBucket::new(RenderingDrawingCategory), + ProfilerBucket::new(RenderingPrepBuffCategory), + ProfilerBucket::new(RenderingCategory), ]; ProfilerCategory::check_order(&buckets); @@ -93,8 +104,8 @@ impl ProfilerCategory { // ensure that the order of the buckets matches the order of the enum categories fn check_order(vec: &ProfilerBuckets) { - for &(category, _) in vec.iter() { - if category != vec[category as uint].first() { + for (i, bucket) in vec.iter().enumerate() { + if bucket.category as uint != i { fail!("Enum category does not match bucket index. This is a bug."); } } @@ -141,14 +152,11 @@ impl Profiler { fn handle_msg(&mut self, msg: ProfilerMsg) { match msg { - TimeMsg(category, t) => match self.buckets[category as uint] { - //TODO(tkuehn): would be nice to have tuple.second_mut() - (_, ref mut data) => data.push(t), - }, + TimeMsg(category, t) => self.buckets[category as uint].data.push(t), PrintMsg => match self.last_msg { // only print if more data has arrived since the last printout Some(TimeMsg(*)) => self.print_buckets(), - _ => {} + _ => () }, }; self.last_msg = Some(msg); @@ -158,10 +166,7 @@ impl Profiler { println(fmt!("%31s %15s %15s %15s %15s %15s", "_category_", "_mean (ms)_", "_median (ms)_", "_min (ms)_", "_max (ms)_", "_bucket size_")); - for bucket in self.buckets.mut_iter() { - let (category, data) = match *bucket { - (category, ref mut data) => (category, data), - }; + for &ProfilerBucket { category: ref category, data: ref mut data } in self.buckets.mut_iter() { tim_sort(*data); let data_len = data.len(); if data_len > 0 { From 4c1648f8f5e34a5ed1d520605440757d74ebfe2c Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Tue, 27 Aug 2013 22:22:03 -0400 Subject: [PATCH 2/6] add tests for profiler buckets --- src/components/util/time.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index 381aa6c1ba7..470769bb0af 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -80,9 +80,8 @@ impl ProfilerCategory { } // enumeration of all ProfilerCategory types - // TODO(tkuehn): is there a better way to ensure proper order of categories? fn empty_buckets() -> ProfilerBuckets { - let buckets = [ + [ ProfilerBucket::new(CompositingCategory), ProfilerBucket::new(LayoutQueryCategory), ProfilerBucket::new(LayoutPerformCategory), @@ -96,19 +95,7 @@ impl ProfilerCategory { ProfilerBucket::new(RenderingDrawingCategory), ProfilerBucket::new(RenderingPrepBuffCategory), ProfilerBucket::new(RenderingCategory), - ]; - - ProfilerCategory::check_order(&buckets); - buckets - } - - // ensure that the order of the buckets matches the order of the enum categories - fn check_order(vec: &ProfilerBuckets) { - for (i, bucket) in vec.iter().enumerate() { - if bucket.category as uint != i { - fail!("Enum category does not match bucket index. This is a bug."); - } - } + ] } // some categories are subcategories of LayoutPerformCategory @@ -207,4 +194,14 @@ pub fn time(msg: &str, callback: &fn() -> T) -> T{ return val; } - +#[cfg(test)] +mod test { + // ensure that the order of the buckets matches the order of the enum categories + #[test] + fn check_order() { + let buckets = ProfilerCategory::empty_buckets(); + for (i, bucket) in buckets.iter().enumerate() { + assert!(bucket.category as uint == i); + } + } +} From 048a39d4a5ff9a0a916a9f9542bad309a60f9c96 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Tue, 27 Aug 2013 22:27:31 -0400 Subject: [PATCH 3/6] add reference to issue regarding CTFE function on enum types for length --- src/components/util/time.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index 470769bb0af..ac6bc083fce 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -41,7 +41,7 @@ pub enum ProfilerCategory { RenderingDrawingCategory, RenderingPrepBuffCategory, RenderingCategory, - // hackish but helps prevent errors when adding new categories + // FIXME(rust#8803): workaround for lack of CTFE function on enum types to return length NumBuckets, } struct ProfilerBucket { @@ -56,7 +56,7 @@ impl ProfilerBucket { } } } -// FIXME(rust#5873) this should be initialized by a NumBuckets cast, +// FIXME(rust#5873) this should be initialized by a NumBuckets cast type ProfilerBuckets = [ProfilerBucket, ..13]; pub enum ProfilerMsg { From 47ec798b92f35134d9324d81bd5c8a6fe6ffa094 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Wed, 28 Aug 2013 18:37:34 -0400 Subject: [PATCH 4/6] use HashMap instead of vector for profiler buckets --- src/components/util/time.rs | 71 +++++++++++++++---------------------- 1 file changed, 29 insertions(+), 42 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index ac6bc083fce..a8455a67028 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -8,6 +8,7 @@ use std::cell::Cell; use std::comm::{Port, SharedChan}; use extra::sort::tim_sort; use std::iterator::AdditiveIterator; +use std::hashmap::HashMap; // front-end representation of the profiler used to communicate with the profiler #[deriving(Clone)] @@ -26,7 +27,14 @@ impl ProfilerChan { } } -#[deriving(Eq, Clone)] +pub enum ProfilerMsg { + // Normal message used for reporting time + TimeMsg(ProfilerCategory, float), + // Message used to force print the profiling metrics + PrintMsg, +} + +#[deriving(Eq, Clone, IterBytes)] pub enum ProfilerCategory { CompositingCategory, LayoutQueryCategory, @@ -44,27 +52,7 @@ pub enum ProfilerCategory { // FIXME(rust#8803): workaround for lack of CTFE function on enum types to return length NumBuckets, } -struct ProfilerBucket { - category: ProfilerCategory, - data: ~[float], -} -impl ProfilerBucket { - fn new(category: ProfilerCategory) -> ProfilerBucket { - ProfilerBucket { - category: category, - data: ~[], - } - } -} -// FIXME(rust#5873) this should be initialized by a NumBuckets cast -type ProfilerBuckets = [ProfilerBucket, ..13]; - -pub enum ProfilerMsg { - // Normal message used for reporting time - TimeMsg(ProfilerCategory, float), - // Message used to force print the profiling metrics - PrintMsg, -} +type ProfilerBuckets = HashMap; // back end of the profiler that handles data aggregation and performance metrics pub struct Profiler { @@ -81,21 +69,22 @@ impl ProfilerCategory { // enumeration of all ProfilerCategory types fn empty_buckets() -> ProfilerBuckets { - [ - ProfilerBucket::new(CompositingCategory), - ProfilerBucket::new(LayoutQueryCategory), - ProfilerBucket::new(LayoutPerformCategory), - ProfilerBucket::new(LayoutAuxInitCategory), - ProfilerBucket::new(LayoutSelectorMatchCategory), - ProfilerBucket::new(LayoutTreeBuilderCategory), - ProfilerBucket::new(LayoutMainCategory), - ProfilerBucket::new(LayoutShapingCategory), - ProfilerBucket::new(LayoutDispListBuildCategory), - ProfilerBucket::new(GfxRegenAvailableFontsCategory), - ProfilerBucket::new(RenderingDrawingCategory), - ProfilerBucket::new(RenderingPrepBuffCategory), - ProfilerBucket::new(RenderingCategory), - ] + let mut buckets = HashMap::with_capacity(NumBuckets as uint); + buckets.insert(CompositingCategory, ~[]); + buckets.insert(LayoutQueryCategory, ~[]); + buckets.insert(LayoutPerformCategory, ~[]); + buckets.insert(LayoutAuxInitCategory, ~[]); + buckets.insert(LayoutSelectorMatchCategory, ~[]); + buckets.insert(LayoutTreeBuilderCategory, ~[]); + buckets.insert(LayoutMainCategory, ~[]); + buckets.insert(LayoutShapingCategory, ~[]); + buckets.insert(LayoutDispListBuildCategory, ~[]); + buckets.insert(GfxRegenAvailableFontsCategory, ~[]); + buckets.insert(RenderingDrawingCategory, ~[]); + buckets.insert(RenderingPrepBuffCategory, ~[]); + buckets.insert(RenderingCategory, ~[]); + + buckets } // some categories are subcategories of LayoutPerformCategory @@ -139,7 +128,7 @@ impl Profiler { fn handle_msg(&mut self, msg: ProfilerMsg) { match msg { - TimeMsg(category, t) => self.buckets[category as uint].data.push(t), + TimeMsg(category, t) => self.buckets.get_mut(&category).push(t), PrintMsg => match self.last_msg { // only print if more data has arrived since the last printout Some(TimeMsg(*)) => self.print_buckets(), @@ -153,7 +142,7 @@ impl Profiler { println(fmt!("%31s %15s %15s %15s %15s %15s", "_category_", "_mean (ms)_", "_median (ms)_", "_min (ms)_", "_max (ms)_", "_bucket size_")); - for &ProfilerBucket { category: ref category, data: ref mut data } in self.buckets.mut_iter() { + for (category, data) in self.buckets.mut_iter() { tim_sort(*data); let data_len = data.len(); if data_len > 0 { @@ -200,8 +189,6 @@ mod test { #[test] fn check_order() { let buckets = ProfilerCategory::empty_buckets(); - for (i, bucket) in buckets.iter().enumerate() { - assert!(bucket.category as uint == i); - } + assert!(buckets.len() == NumBuckets as uint); } } From cf4107e262c7dee71382b8400090c376e7bce7fe Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Thu, 29 Aug 2013 00:42:20 -0400 Subject: [PATCH 5/6] use TreeMap instead of HashMap for profiler buckets --- src/components/util/time.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index a8455a67028..58cd8210040 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -8,7 +8,7 @@ use std::cell::Cell; use std::comm::{Port, SharedChan}; use extra::sort::tim_sort; use std::iterator::AdditiveIterator; -use std::hashmap::HashMap; +use extra::treemap::TreeMap; // front-end representation of the profiler used to communicate with the profiler #[deriving(Clone)] @@ -34,7 +34,7 @@ pub enum ProfilerMsg { PrintMsg, } -#[deriving(Eq, Clone, IterBytes)] +#[deriving(Eq, Clone, TotalEq, TotalOrd)] pub enum ProfilerCategory { CompositingCategory, LayoutQueryCategory, @@ -52,7 +52,7 @@ pub enum ProfilerCategory { // FIXME(rust#8803): workaround for lack of CTFE function on enum types to return length NumBuckets, } -type ProfilerBuckets = HashMap; +type ProfilerBuckets = TreeMap; // back end of the profiler that handles data aggregation and performance metrics pub struct Profiler { @@ -69,7 +69,7 @@ impl ProfilerCategory { // enumeration of all ProfilerCategory types fn empty_buckets() -> ProfilerBuckets { - let mut buckets = HashMap::with_capacity(NumBuckets as uint); + let mut buckets = TreeMap::new(); buckets.insert(CompositingCategory, ~[]); buckets.insert(LayoutQueryCategory, ~[]); buckets.insert(LayoutPerformCategory, ~[]); @@ -128,7 +128,7 @@ impl Profiler { fn handle_msg(&mut self, msg: ProfilerMsg) { match msg { - TimeMsg(category, t) => self.buckets.get_mut(&category).push(t), + TimeMsg(category, t) => self.buckets.find_mut(&category).unwrap().push(t), PrintMsg => match self.last_msg { // only print if more data has arrived since the last printout Some(TimeMsg(*)) => self.print_buckets(), @@ -142,8 +142,10 @@ impl Profiler { println(fmt!("%31s %15s %15s %15s %15s %15s", "_category_", "_mean (ms)_", "_median (ms)_", "_min (ms)_", "_max (ms)_", "_bucket size_")); - for (category, data) in self.buckets.mut_iter() { - tim_sort(*data); + for (category, data) in self.buckets.iter() { + // FIXME(XXX): TreeMap currently lacks mut_iter() + let mut data = data.clone(); + tim_sort(data); let data_len = data.len(); if data_len > 0 { let (mean, median, &min, &max) = From 1d2c038954d58e251305bf99a94b5c2c952c4a08 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Thu, 29 Aug 2013 00:45:11 -0400 Subject: [PATCH 6/6] reorder some struct declarations in profiler --- src/components/util/time.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/util/time.rs b/src/components/util/time.rs index 58cd8210040..a847bef12c7 100644 --- a/src/components/util/time.rs +++ b/src/components/util/time.rs @@ -52,14 +52,6 @@ pub enum ProfilerCategory { // FIXME(rust#8803): workaround for lack of CTFE function on enum types to return length NumBuckets, } -type ProfilerBuckets = TreeMap; - -// back end of the profiler that handles data aggregation and performance metrics -pub struct Profiler { - port: Port, - buckets: ProfilerBuckets, - last_msg: Option, -} impl ProfilerCategory { // convenience function to not have to cast every time @@ -99,6 +91,15 @@ impl ProfilerCategory { } } +type ProfilerBuckets = TreeMap; + +// back end of the profiler that handles data aggregation and performance metrics +pub struct Profiler { + port: Port, + buckets: ProfilerBuckets, + last_msg: Option, +} + impl Profiler { pub fn create(port: Port) { let port = Cell::new(port);