Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark all Option types non_exhaustive #181

Merged
merged 1 commit into from
Jun 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]
### Added
- Flame chart mode. Flame charts put the passage of time on the x-axis instead of the alphabet. [#125](https://github.com/jonhoo/inferno/pull/125)

- Flame chart mode. Flame charts put the passage of time on the x-axis instead of the alphabet. [#125](https://github.com/jonhoo/inferno/pull/125)
- `cargo hack` to check that all features compile. [#181](https://github.com/jonhoo/inferno/pull/181)

### Changed

- All `Options` are now marked as `#[non_exhaustive]` so that we can
add options without making that a breaking change. This also makes
feature-dependent fields (like `func_nameattr` on `flamegraph`) okay.
Unfortunately, it also means that function record update syntax won't
work any more (`Options { ..., ..Default::default() }`). See
https://github.com/rust-lang/rust/issues/70564#issuecomment-647031324
for details. [#181](https://github.com/jonhoo/inferno/pull/181)

### Removed

## [0.9.9] - 2020-06-03
Expand Down
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ required-features = ["cli"]
[[bench]]
name = "collapse"
harness = false
required-features = ["multithreaded"]

[[bench]]
name = "flamegraph"
harness = false
required-features = ["multithreaded"]
30 changes: 21 additions & 9 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
jobs:
- template: default.yml@templates
parameters:
codecov_token: $(CODECOV_TOKEN_SECRET)
minrust: 1.40.0
env:
RUST_BACKTRACE: 1
setup:
- checkout: self
submodules: recursive
- template: default.yml@templates
parameters:
codecov_token: $(CODECOV_TOKEN_SECRET)
minrust: 1.40.0
env:
RUST_BACKTRACE: 1
setup:
- checkout: self
submodules: recursive
- job: features
displayName: "Check feature combinations"
pool:
vmImage: ubuntu-latest
steps:
- template: install-rust.yml@templates
parameters:
rust: stable
- script: cargo install cargo-hack
displayName: install cargo-hack
- script: cargo hack
displayName: cargo hack --feature-powerset check --all-targets

resources:
repositories:
Expand Down
2 changes: 1 addition & 1 deletion benches/flamegraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ macro_rules! flamegraph_benchmarks {

flamegraph_benchmarks! {
flamegraph: ("tests/data/collapse-perf/results/example-perf-stacks-collapsed.txt",
Options { reverse_stack_order: true, ..Default::default() })
{ let mut opt = Options::default(); opt.reverse_stack_order = true; opt })
}
11 changes: 4 additions & 7 deletions src/bin/collapse-dtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,10 @@ struct Opt {

impl Opt {
fn into_parts(self) -> (Option<PathBuf>, Options) {
(
self.infile,
Options {
includeoffset: self.includeoffset,
nthreads: self.nthreads,
},
)
let mut options = Options::default();
options.includeoffset = self.includeoffset;
options.nthreads = self.nthreads;
(self.infile, options)
}
}

Expand Down
9 changes: 3 additions & 6 deletions src/bin/collapse-guess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,9 @@ struct Opt {

impl Opt {
fn into_parts(self) -> (Option<PathBuf>, Options) {
(
self.infile,
Options {
nthreads: self.nthreads,
},
)
let mut options = Options::default();
options.nthreads = self.nthreads;
(self.infile, options)
}
}

Expand Down
21 changes: 9 additions & 12 deletions src/bin/collapse-perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,15 @@ struct Opt {

impl Opt {
fn into_parts(self) -> (Option<PathBuf>, Options) {
(
self.infile,
Options {
include_pid: self.pid,
include_tid: self.tid,
include_addrs: self.addrs,
annotate_jit: self.jit || self.all,
annotate_kernel: self.kernel || self.all,
event_filter: self.event_filter,
nthreads: self.nthreads,
},
)
let mut options = Options::default();
options.include_pid = self.pid;
options.include_tid = self.tid;
options.include_addrs = self.addrs;
options.annotate_jit = self.jit || self.all;
options.annotate_kernel = self.kernel || self.all;
options.event_filter = self.event_filter;
options.nthreads = self.nthreads;
(self.infile, options)
}
}

Expand Down
9 changes: 3 additions & 6 deletions src/bin/collapse-sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,9 @@ struct Opt {

impl Opt {
fn into_parts(self) -> (Option<PathBuf>, Options) {
(
self.infile,
Options {
no_modules: self.no_modules,
},
)
let mut options = Options::default();
options.no_modules = self.no_modules;
(self.infile, options)
}
}

Expand Down
9 changes: 3 additions & 6 deletions src/bin/collapse-vtune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,9 @@ struct Opt {

impl Opt {
fn into_parts(self) -> (Option<PathBuf>, Options) {
(
self.infile,
Options {
no_modules: self.no_modules,
},
)
let mut options = Options::default();
options.no_modules = self.no_modules;
(self.infile, options)
}
}

Expand Down
55 changes: 25 additions & 30 deletions src/bin/flamegraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,36 +387,31 @@ mod tests {
];
let opt = Opt::from_iter_safe(args).unwrap();
let (infiles, options) = opt.into_parts();
let expected_options = Options {
colors: Palette::from_str("purple").unwrap(),
search_color: color::SearchColor::from_str("#203040").unwrap(),
title: "Test Title".to_string(),
image_width: Some(100),
frame_height: 500,
min_width: 90.1,
font_type: "Helvetica".to_string(),
font_size: 13,
font_width: 10.5,
text_truncate_direction: TextTruncateDirection::Right,
count_name: "test count name".to_string(),
name_type: "test name type".to_string(),
factor: 0.1,
notes: "Test notes".to_string(),
subtitle: Some("Test Subtitle".to_string()),
bgcolors: Some(color::BackgroundColor::Blue),
hash: true,
palette_map: Default::default(),
#[cfg(feature = "nameattr")]
func_frameattrs: Default::default(),
direction: Direction::Inverted,
negate_differentials: true,
pretty_xml: true,
no_sort: false,
reverse_stack_order: true,
no_javascript: true,
color_diffusion: false,
flame_chart: false,
};
let mut expected_options = Options::default();
expected_options.colors = Palette::from_str("purple").unwrap();
expected_options.search_color = color::SearchColor::from_str("#203040").unwrap();
expected_options.title = "Test Title".to_string();
expected_options.image_width = Some(100);
expected_options.frame_height = 500;
expected_options.min_width = 90.1;
expected_options.font_type = "Helvetica".to_string();
expected_options.font_size = 13;
expected_options.font_width = 10.5;
expected_options.text_truncate_direction = TextTruncateDirection::Right;
expected_options.count_name = "test count name".to_string();
expected_options.name_type = "test name type".to_string();
expected_options.factor = 0.1;
expected_options.notes = "Test notes".to_string();
expected_options.subtitle = Some("Test Subtitle".to_string());
expected_options.bgcolors = Some(color::BackgroundColor::Blue);
expected_options.hash = true;
expected_options.direction = Direction::Inverted;
expected_options.negate_differentials = true;
expected_options.pretty_xml = true;
expected_options.no_sort = false;
expected_options.reverse_stack_order = true;
expected_options.no_javascript = true;
expected_options.color_diffusion = false;

assert_eq!(options, expected_options);
assert_eq!(infiles.len(), 2, "expected 2 input files");
Expand Down
3 changes: 3 additions & 0 deletions src/collapse/common.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::borrow::Cow;
use std::io;
#[cfg(feature = "multithreaded")]
use std::mem;
#[cfg(feature = "multithreaded")]
use std::sync::Arc;

use ahash::AHashMap;
Expand Down Expand Up @@ -33,6 +35,7 @@ pub(crate) const DEFAULT_NSTACKS_PER_JOB: usize = 100;
/// A guess at the number of bytes contained in any given stack of any given format.
/// Used to calculate the initial capacity of the vector used for sending input
/// data across threads.
#[cfg(feature = "multithreaded")]
const NBYTES_PER_STACK_GUESS: usize = 1024;

const RUST_HASH_LENGTH: usize = 17;
Expand Down
1 change: 1 addition & 0 deletions src/collapse/dtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::collapse::common::{self, CollapsePrivate, Occurrences};

/// `dtrace` folder configuration options.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct Options {
/// Include function offset (except leafs).
///
Expand Down
1 change: 1 addition & 0 deletions src/collapse/guess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const LINES_PER_ITERATION: usize = 10;

/// Folder configuration options.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct Options {
/// The number of threads to use.
///
Expand Down
1 change: 1 addition & 0 deletions src/collapse/perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mod logging {

/// `perf` folder configuration options.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct Options {
/// Annotate JIT functions with a `_[j]` suffix.
///
Expand Down
1 change: 1 addition & 0 deletions src/collapse/sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ static END_LINE: &str = "Total number in stack";

/// `sample` folder configuration options.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct Options {
/// Don't include modules with function names.
///
Expand Down
1 change: 1 addition & 0 deletions src/collapse/vtune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ static HEADER: &str = "Function Stack,CPU Time:Self,Module";

/// `vtune` folder configuration options.
#[derive(Clone, Debug)]
#[non_exhaustive]
pub struct Options {
/// Don't include modules with function names.
///
Expand Down
1 change: 1 addition & 0 deletions src/flamegraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub mod defaults {

/// Configure the flame graph.
#[derive(Debug, PartialEq)]
#[non_exhaustive]
pub struct Options<'a> {
/// The color palette to use when plotting.
pub colors: color::Palette,
Expand Down
28 changes: 10 additions & 18 deletions tests/collapse-dtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,11 @@ fn collapse_dtrace_compare_to_upstream() {
fn collapse_dtrace_compare_to_upstream_with_offsets() {
let test_file = "./flamegraph/example-dtrace-stacks.txt";
let result_file = "./tests/data/collapse-dtrace/results/dtrace-example-offsets.txt";
test_collapse_dtrace(
test_file,
result_file,
Options {
includeoffset: true,
..Default::default()
},
)
.unwrap()

let mut options = Options::default();
options.includeoffset = true;

test_collapse_dtrace(test_file, result_file, options).unwrap()
}

#[test]
Expand All @@ -80,15 +76,11 @@ fn collapse_dtrace_compare_to_flamegraph_bug() {
// https://github.com/brendangregg/FlameGraph/issues/202
let test_file = "./tests/data/collapse-dtrace/flamegraph-bug.txt";
let result_file = "./tests/data/collapse-dtrace/results/flamegraph-bug.txt";
test_collapse_dtrace(
test_file,
result_file,
Options {
includeoffset: true,
..Default::default()
},
)
.unwrap()

let mut options = Options::default();
options.includeoffset = true;

test_collapse_dtrace(test_file, result_file, options).unwrap()
}

#[test]
Expand Down
6 changes: 5 additions & 1 deletion tests/collapse-sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ fn collapse_sample_default() {
fn collapse_sample_no_modules() {
let test_file = "./tests/data/collapse-sample/sample.txt";
let result_file = "./tests/data/collapse-sample/results/sample-no-modules.txt";
test_collapse_sample(test_file, result_file, Options { no_modules: true }).unwrap()

let mut options = Options::default();
options.no_modules = true;

test_collapse_sample(test_file, result_file, options).unwrap()
}

#[test]
Expand Down
6 changes: 5 additions & 1 deletion tests/collapse-vtune.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ fn collapse_vtune_default() {
fn collapse_vtune_no_modules() {
let test_file = "./tests/data/collapse-vtune/vtune.csv";
let result_file = "./tests/data/collapse-vtune/results/vtune-no-modules.txt";
test_collapse_vtune(test_file, result_file, Options { no_modules: true }).unwrap()

let mut options = Options::default();
options.no_modules = true;

test_collapse_vtune(test_file, result_file, options).unwrap()
}

#[test]
Expand Down
Loading