Address review comments

Author: Kobzol

collector/benchmarks/README.md

@@ -7,20 +7,33 @@ The suite changes over time. Sometimes the code for a benchmark is updated, in
 which case a small suffix will be added (starting with "-2", then "-3", and so
 on.)
 
-## Real programs that are important
+There are two categories of benchmarks, **Primary** and **Secondary**.
+
+## Primary
 
 These are real programs that are important in some way, and worth tracking.
+They mostly consist of real-world crates.
 
 - **cargo**: The Rust package manager. An important program in the Rust
   ecosystem.
 - **clap-rs**: A command line argument parser. A crate used by many Rust
   programs.
 - **cranelift-codegen**: The largest crate from a code generator. Used by
   Firefox.
+- **diesel**: A type save SQL query builder. Utilizes the type system to
+  ensure a lot of invariants. Stresses anything related to resolving
+  trait bounds, by having a lot of trait impls for a large number of different
+  types.
+- **encoding**: Character encoding support. Contains some large tables.
 - **futures**: A futures implementation. Used by many Rust programs.
 - **helloworld**: A trivial program. Gives a lower bound on compile time.
+- **html5ever**: An HTML parser. Stresses macro parsing code significantly.
 - **hyper-2**: A fairly large crate. Utilizes async/await, and used by
   many Rust programs.
+- **inflate**: An old implementation of the DEFLATE algorithm. Stresses the
+  compiler in certain ways.
+- **keccak**: A cryptography algorithm. Contains a very high number of locals
+  and basic blocks.
 - **piston-image**: A modular game engine. An interesting Rust program.
 - **regex**: A regular expression parser. Used by many Rust programs.
 - **ripgrep**: A line-oriented search tool. A widely-used utility.
@@ -33,40 +46,20 @@ These are real programs that are important in some way, and worth tracking.
   ecosystem.
 - **tokio-webpush-simple**: A simple web server built with tokio. Uses futures
   a lot.
-- **webrender**: A web renderer. Used by Firefox and Servo.
-- **webrender-wrench**: WebRender's test bench. An executable pulling in large
-  dependencies.
-
-## Real programs that stress the compiler
-
-These are real programs that are known to stress the compiler in interesting
-ways.
-
-- **diesel**: A type save SQL query builder. Utilizes the type system to
-  ensure a lot of invariants. Stresses anything related to resolving
-  trait bounds, by having a lot of trait impls for a large number of different
-  types.
-- **encoding**: Character encoding support. Contains some large tables.
-- **html5ever**: An HTML parser. Stresses macro parsing code significantly.
-- **inflate**: An old implementation of the DEFLATE algorithm. Stresses the
-  compiler in certain ways.
-- **keccak**: A cryptography algorithm. Contains a very high number of locals
-  and basic blocks.
 - **ucd**: A Unicode crate. Contains large statics that
   [stress](https://github.com/rust-lang/rust/issues/53643) the borrow checker's
   implementation of NLL.
 - **unicode_normalization**: Unicode character composition and decomposition
   utilities. Uses huge `match` statements that stress the compiler in unusual
   ways.
-- **wg-grammar**: A parser generator.
-  [Stresses](https://github.com/rust-lang/rust/issues/58178) the borrow
-  checker's implementation of NLL.
+- **webrender**: A web renderer. Used by Firefox and Servo.
+- **webrender-wrench**: WebRender's test bench. An executable pulling in large
+  dependencies.
 
-## Artificial stress tests
+## Secondary
 
-These are artificial programs that stress one particular aspect of the
-compiler. Some are entirely artificial, and some are extracted from real
-programs.
+These are either artificial programs or real crates that stress one particular aspect of the
+compiler in interesting ways.
 
 - **await-call-tree**: A tree of async fns that await each other, creating a
   large type composed of many repeated `impl Future` types. Such types caused
@@ -85,6 +78,8 @@ programs.
 - **deep-vector**: A test containing a single large vector of zeroes, which
   caused [poor performance](https://github.com/rust-lang/rust/issues/20936) in
   the past.
+- **derive**: A large amount of simple structs with a `#[derive]` attribute for common built-in traits such as Copy and Debug.
+- **externs**: A large amount of extern functions has caused [slowdowns in the past](https://github.com/rust-lang/rust/pull/78448).
 - **issue-46449**: A small program that caused [poor
   performance](https://github.com/rust-lang/rust/issues/46449) in the past.
 - **issue-58319**: A small program that caused [poor
@@ -103,7 +98,7 @@ programs.
 - **projection-caching**: A small program that causes extremely, deeply nested
   types which stress the trait system's projection cache. Removing that cache
   resulted in hours long compilations for some programs using futures,
-  actix-web and other libraries with similiarly nested type combinators.
+  actix-web and other libraries with similarly nested type combinators.
 - **regression-31157**: A small program that caused a [large performance
   regression](https://github.com/rust-lang/rust/issues/31157) from the past.
 - **token-stream-stress**: Constructs a long token stream much like the `quote`
@@ -122,5 +117,6 @@ programs.
 - **wf-projection-stress-65510**: A stress test which showcases [quadratic
   behavior](https://github.com/rust-lang/rust/issues/65510) (in the number of
   associated type bounds).
-- **externs**: A large amount of extern functions has caused [slowdowns in the past](https://github.com/rust-lang/rust/pull/78448).
-- **derive**: A large amount of simple structs with a `#[derive]` attribute for common built-in traits such as Copy and Debug.
+- **wg-grammar**: A parser generator.
+  [Stresses](https://github.com/rust-lang/rust/issues/58178) the borrow
+  checker's implementation of NLL.

collector/benchmarks/inflate/perf-config.json

@@ -1,3 +1,4 @@
 {
-    "supports_stable": true
+    "supports_stable": true,
+    "category": "primary"
 }

collector/benchmarks/rust-mozjs/perf-config.json renamed to collector/benchmarks/keccak/perf-config.json

@@ -1,4 +1,3 @@
 {
-    "touch_file": "src/lib.rs",
-    "category": "primary"
+    "touch_file": "src/lib.rs"
 }

collector/benchmarks/native-tls-0.1.5/perf-config.json

@@ -1,4 +1,5 @@
 {
     "supports_stable": true,
-    "touch_file": "src/main.rs"
+    "touch_file": "src/main.rs",
+    "category": "primary"
 }

collector/benchmarks/tokio-webpush-simple/perf-config.json

@@ -1,4 +1,3 @@
 {
-    "touch_file": "src/lib.rs",
-    "category": "primary"
+    "touch_file": "src/lib.rs"
 }

collector/benchmarks/wg-grammar/perf-config.json

@@ -112,17 +112,25 @@ fn default_runs() -> usize {
 
 #[derive(Debug, Clone, serde::Deserialize)]
 #[serde(rename_all = "lowercase")]
-enum BenchmarkCategory {
+enum Category {
     Primary,
     Secondary
 }
 
-impl Default for BenchmarkCategory {
+impl Default for Category {
     fn default() -> Self {
         Self::Secondary
     }
 }
 
+impl fmt::Display for Category {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Category::Primary => f.write_str("primary"),
+            Category::Secondary => f.write_str("secondary")
+        }
+    }
+}
 /// This is the internal representation of an individual benchmark's
 /// perf-config.json file.
 #[derive(Debug, Clone, serde::Deserialize)]
@@ -145,7 +153,7 @@ struct BenchmarkConfig {
     touch_file: Option<String>,
 
     #[serde(default)]
-    category: BenchmarkCategory
+    category: Category
 }
 
 impl Default for BenchmarkConfig {
@@ -158,7 +166,7 @@ impl Default for BenchmarkConfig {
             runs: default_runs(),
             supports_stable: false,
             touch_file: None,
-            category: BenchmarkCategory::Secondary
+            category: Category::Secondary
         }
     }
 }
@@ -1256,11 +1264,8 @@ impl Benchmark {
         self.config.supports_stable
     }
 
-    pub fn category_name(&self) -> &str {
-        match self.config.category {
-            BenchmarkCategory::Primary => "primary",
-            BenchmarkCategory::Secondary => "secondary"
-        }
+    pub fn category(&self) -> &Category {
+        &self.config.category
     }
 
     #[cfg(windows)]

collector/src/execute.rs

@@ -238,7 +238,7 @@ fn bench(
         measure_and_record(
             &benchmark.name,
             benchmark.supports_stable(),
-            benchmark.category_name(),
+            &benchmark.category().to_string(),
             &|| {
                 eprintln!(
                     "{}",

collector/src/main.rs

@@ -9,6 +9,7 @@ The following is a glossary of domain specific terminology. Although benchmarks
 * **scenario**: The scenario under which a user is compiling their code. Currently, this is the incremental cache state and an optional change in the source since last compilation (e.g., full incremental cache and a `println!` statement is added).  
 * **metric**: a name of a quantifiable metric being measured (e.g., instruction count)
 * **artifact**: a specific version of rustc (usually a commit sha or some sort of human readable "tag" like 1.51.0)
+* **category**: a high-level group of benchmarks. Currently, there are two categories, primary (mostly real-world crates) and secondary (mostly stress tests).
 
 ## Benchmarks