Skip to content

Commit 0dabf63

Browse files
mattssegakonst
andauthored
perf: remove redundant config usage (#1908)
* fix: remove redundant config usage * test: adjust fixture to have correct line number Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
1 parent e43a1dd commit 0dabf63

File tree

8 files changed

+46
-23
lines changed

8 files changed

+46
-23
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cli/src/cmd/forge/test.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ use std::{
3232
thread,
3333
time::Duration,
3434
};
35+
use tracing::trace;
3536
use watchexec::config::{InitConfig, RuntimeConfig};
3637
use yansi::Paint;
3738

@@ -100,26 +101,26 @@ pub struct Filter {
100101
}
101102

102103
impl Filter {
103-
pub fn with_merged_config(&self) -> Self {
104-
let config = Config::load();
104+
pub fn with_merged_config(&self, config: &Config) -> Self {
105105
let mut filter = self.clone();
106106
if filter.test_pattern.is_none() {
107-
filter.test_pattern = config.test_pattern.map(|p| p.into());
107+
filter.test_pattern = config.test_pattern.clone().map(|p| p.into());
108108
}
109109
if filter.test_pattern_inverse.is_none() {
110-
filter.test_pattern_inverse = config.test_pattern_inverse.map(|p| p.into());
110+
filter.test_pattern_inverse = config.test_pattern_inverse.clone().map(|p| p.into());
111111
}
112112
if filter.contract_pattern.is_none() {
113-
filter.contract_pattern = config.contract_pattern.map(|p| p.into());
113+
filter.contract_pattern = config.contract_pattern.clone().map(|p| p.into());
114114
}
115115
if filter.contract_pattern_inverse.is_none() {
116-
filter.contract_pattern_inverse = config.contract_pattern_inverse.map(|p| p.into());
116+
filter.contract_pattern_inverse =
117+
config.contract_pattern_inverse.clone().map(|p| p.into());
117118
}
118119
if filter.path_pattern.is_none() {
119-
filter.path_pattern = config.path_pattern;
120+
filter.path_pattern = config.path_pattern.clone();
120121
}
121122
if filter.path_pattern_inverse.is_none() {
122-
filter.path_pattern_inverse = config.path_pattern_inverse;
123+
filter.path_pattern_inverse = config.path_pattern_inverse.clone();
123124
}
124125
filter
125126
}
@@ -281,8 +282,8 @@ impl TestArgs {
281282
}
282283

283284
/// Returns the flattened [`Filter`] arguments merged with [`Config`]
284-
pub fn filter(&self) -> Filter {
285-
self.filter.with_merged_config()
285+
pub fn filter(&self, config: &Config) -> Filter {
286+
self.filter.with_merged_config(config)
286287
}
287288

288289
/// Returns the currently configured [Config] and the extracted [EvmOpts] from that config
@@ -318,6 +319,7 @@ impl Cmd for TestArgs {
318319
type Output = TestOutcome;
319320

320321
fn run(self) -> eyre::Result<Self::Output> {
322+
trace!(target: "forge::test", "executing test command");
321323
custom_run(self, true)
322324
}
323325
}
@@ -468,7 +470,7 @@ pub fn custom_run(args: TestArgs, include_fuzz_tests: bool) -> eyre::Result<Test
468470
..Default::default()
469471
};
470472
let fuzzer = proptest::test_runner::TestRunner::new(cfg);
471-
let mut filter = args.filter();
473+
let mut filter = args.filter(&config);
472474

473475
// Set up the project
474476
let project = config.project()?;
@@ -589,6 +591,7 @@ fn test(
589591
include_fuzz_tests: bool,
590592
gas_reporting: bool,
591593
) -> eyre::Result<TestOutcome> {
594+
trace!(target: "forge::test", "running all tests");
592595
if runner.count_filtered_tests(&filter) == 0 {
593596
let filter_str = filter.to_string();
594597
if filter_str.is_empty() {

cli/src/cmd/forge/verify.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use ethers::{
1818
},
1919
};
2020
use eyre::{eyre, Context};
21-
use foundry_config::{Chain, Config, SolcReq};
21+
use foundry_config::{find_project_root_path, Chain, Config, SolcReq};
2222
use foundry_utils::Retry;
2323
use futures::FutureExt;
2424
use once_cell::sync::Lazy;
@@ -198,7 +198,7 @@ impl VerifyArgs {
198198
};
199199

200200
let project = build_args.project()?;
201-
let config = Config::load();
201+
let config = Config::load_with_root(find_project_root_path().unwrap());
202202

203203
if self.contract.path.is_none() && !config.cache {
204204
eyre::bail!(

cli/src/cmd/forge/watch.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,17 @@ pub async fn watch_test(args: TestArgs) -> eyre::Result<()> {
132132
runtime.command(cmd.clone());
133133
let wx = Watchexec::new(init, runtime.clone())?;
134134

135-
let filter = args.filter();
135+
let config: Config = args.build_args().into();
136+
137+
let filter = args.filter(&config);
138+
136139
// marker to check whether to override the command
137140
let no_reconfigure = filter.pattern.is_some() ||
138141
filter.test_pattern.is_some() ||
139142
filter.path_pattern.is_some() ||
140143
filter.contract_pattern.is_some() ||
141144
args.watch.run_all;
142145

143-
let config: Config = args.build_args().into();
144146
let state = WatchTestState {
145147
project_root: config.__root.0,
146148
no_reconfigure,

cli/src/compile.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,15 @@ If you are in a subdirectory in a Git repository, try adding `--root .`"#,
138138
}
139139

140140
let now = std::time::Instant::now();
141-
tracing::trace!(target : "forge_compile", "start compiling project");
141+
tracing::trace!(target : "forge::compile", "start compiling project");
142142

143143
let output = term::with_spinner_reporter(|| f(project))?;
144144

145145
let elapsed = now.elapsed();
146-
tracing::trace!(target : "forge_compile", "finished compiling after {:?}", elapsed);
146+
tracing::trace!(target : "forge::compile", "finished compiling after {:?}", elapsed);
147147

148148
if output.has_compiler_errors() {
149+
tracing::warn!(target: "forge::compile", "compiled with errors");
149150
eyre::bail!(output.to_string())
150151
} else if output.is_unchanged() {
151152
println!("No files changed, compilation skipped");

cli/tests/fixtures/can_set_yul_optimizer.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Error:
1010
0: 
1111

1212
Location:
13-
cli/src/compile.rs:149
13+
cli/src/compile.rs:150
1414

1515
Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
1616
Run with RUST_BACKTRACE=full to include source snippets.

config/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ regex = "1.5.5"
2525
globset = "0.4.8"
2626
walkdir = "2.3.2"
2727
toml_edit = "0.14.3"
28+
tracing = "0.1"
2829

2930
[target.'cfg(target_os = "windows")'.dependencies]
3031
path-slash = "0.1.4"

config/src/lib.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ pub use chain::Chain;
4646
// reexport so cli types can implement `figment::Provider` to easily merge compiler arguments
4747
pub use figment;
4848
use regex::Regex;
49+
use tracing::trace;
4950

5051
/// Foundry configuration
5152
///
@@ -343,6 +344,7 @@ impl Config {
343344
/// let config = Config::from_provider(figment);
344345
/// ```
345346
pub fn from_provider<T: Provider>(provider: T) -> Self {
347+
trace!("load config with provider: {:?}", provider.metadata());
346348
match Self::try_from(provider) {
347349
Ok(config) => config,
348350
Err(errors) => {
@@ -1805,6 +1807,7 @@ impl<'a> RemappingsProvider<'a> {
18051807
/// - Environment variables
18061808
/// - CLI parameters
18071809
fn get_remappings(&self, remappings: Vec<Remapping>) -> Result<Vec<Remapping>, Error> {
1810+
trace!("get all remappings from {:?}", self.root);
18081811
/// prioritizes remappings that are closer: shorter `path`
18091812
/// - ("a", "1/2") over ("a", "1/2/3")
18101813
fn insert_closest(mappings: &mut HashMap<String, PathBuf>, key: String, path: PathBuf) {
@@ -1848,7 +1851,14 @@ impl<'a> RemappingsProvider<'a> {
18481851
insert_closest(&mut lib_remappings, r.name, r.path.into());
18491852
}
18501853
// use auto detection for all libs
1851-
for r in self.lib_paths.iter().map(|lib| self.root.join(lib)).flat_map(Remapping::find_many)
1854+
for r in self
1855+
.lib_paths
1856+
.iter()
1857+
.map(|lib| self.root.join(lib))
1858+
.inspect(|lib| {
1859+
trace!("find all remappings in lib path: {:?}", lib);
1860+
})
1861+
.flat_map(Remapping::find_many)
18521862
{
18531863
// this is an additional safety check for weird auto-detected remappings
18541864
if ["lib/", "src/", "contracts/"].contains(&r.name.as_str()) {
@@ -1872,8 +1882,14 @@ impl<'a> RemappingsProvider<'a> {
18721882

18731883
/// Returns all remappings declared in foundry.toml files of libraries
18741884
fn lib_foundry_toml_remappings(&self) -> impl Iterator<Item = Remapping> + '_ {
1875-
self.lib_paths.iter().map(|p| self.root.join(p)).flat_map(foundry_toml_dirs).flat_map(
1876-
|lib: PathBuf| {
1885+
self.lib_paths
1886+
.iter()
1887+
.map(|p| self.root.join(p))
1888+
.flat_map(foundry_toml_dirs)
1889+
.inspect(|lib| {
1890+
trace!("find all remappings of nested foundry.toml lib: {:?}", lib);
1891+
})
1892+
.flat_map(|lib: PathBuf| {
18771893
// load config, of the nested lib if it exists
18781894
let config = Config::load_with_root(&lib).sanitized();
18791895

@@ -1903,8 +1919,7 @@ impl<'a> RemappingsProvider<'a> {
19031919
remappings.push(r);
19041920
}
19051921
remappings
1906-
},
1907-
)
1922+
})
19081923
}
19091924
}
19101925

0 commit comments

Comments
 (0)