Skip to content

Commit

Permalink
perf: don't request unnecessary output (#231)
Browse files Browse the repository at this point in the history
We're currently requesting all `evm.bytecode` (and
`evm.deployedBytecode`) output by default. This includes
`evm.bytecode.generatedSources`, introduced in solc 0.8.0, which is a
list of potentially huge Yul AST objects that is completely unused in
Foundry.

Only request the `Compact*` bytecode fields in the defaults.

This cuts down a clean `forge build`:
- from 2:20m to 1:30m (-36%) on
[superform-xyz/superform-core](https://github.com/superform-xyz/superform-core)
- from 1:39m to 1:29m (-10%) on
[sablier-labs/v2-core](https://github.com/sablier-labs/v2-core)
- from 54.5s to 45.7s (-20%) on
[sablier-labs/v2-core](https://github.com/sablier-labs/v2-core)
(FOUNDRY_PROFILE=lite which is `optimizer = false`)

It may have a larger impact when compiling with >=0.8.28, because the
cache storing this data was removed in that version
(ethereum/solidity@3c5e46b),
or when optimizations are disabled as more IR will be generated and
output to JSON.

I verified that these inputs are accepted by solidity 0.4.14, 0.6.3,
0.8.28, and vyper 0.3.10, 0.4.0. All of these outputs are supported by
all of them except for vyper which needs to be normalized.

Drive-by: buffer stdin when writing to the solc subprocess.
  • Loading branch information
DaniPopes authored Dec 4, 2024
1 parent 256918a commit 2199839
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 101 deletions.
63 changes: 20 additions & 43 deletions crates/artifacts/solc/src/output_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,6 @@ pub type FileOutputSelection = BTreeMap<String, Vec<String>>;
/// Note that using a using `evm`, `evm.bytecode`, `ewasm`, etc. will select
/// every target part of that output. Additionally, `*` can be used as a
/// wildcard to request everything.
///
/// The default output selection is
///
/// ```json
/// {
/// "*": {
/// "*": [
/// "abi",
/// "evm.bytecode",
/// "evm.deployedBytecode",
/// "evm.methodIdentifiers"
/// ],
/// "": [
/// "ast"
/// ]
/// }
/// }
/// ```
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize)]
#[serde(transparent)]
pub struct OutputSelection(pub BTreeMap<String, FileOutputSelection>);
Expand All @@ -88,31 +70,18 @@ impl OutputSelection {
.into()
}

/// Default output selection for compiler output:
///
/// `{ "*": { "*": [ "*" ], "": [
/// "abi","evm.bytecode","evm.deployedBytecode","evm.methodIdentifiers"] } }`
///
/// Which enables it for all files and all their contracts ("*" wildcard)
/// Default output selection.
pub fn default_output_selection() -> Self {
BTreeMap::from([("*".to_string(), Self::default_file_output_selection())]).into()
}

/// Default output selection for a single file:
///
/// `{ "*": [ "*" ], "": [
/// "abi","evm.bytecode","evm.deployedBytecode","evm.methodIdentifiers"] }`
/// Default file output selection.
///
/// Which enables it for all the contracts in the file ("*" wildcard)
/// Uses [`ContractOutputSelection::basic`].
pub fn default_file_output_selection() -> FileOutputSelection {
BTreeMap::from([(
"*".to_string(),
vec![
"abi".to_string(),
"evm.bytecode".to_string(),
"evm.deployedBytecode".to_string(),
"evm.methodIdentifiers".to_string(),
],
ContractOutputSelection::basic().iter().map(ToString::to_string).collect(),
)])
}

Expand Down Expand Up @@ -146,7 +115,7 @@ impl OutputSelection {
}

/// Returns true if this output selection is a subset of the other output selection.
/// TODO: correctly process wildcard keys to reduce false negatives
// TODO: correctly process wildcard keys to reduce false negatives
pub fn is_subset_of(&self, other: &Self) -> bool {
self.0.iter().all(|(file, selection)| {
other.0.get(file).is_some_and(|other_selection| {
Expand Down Expand Up @@ -229,16 +198,24 @@ pub enum ContractOutputSelection {

impl ContractOutputSelection {
/// Returns the basic set of contract level settings that should be included in the `Contract`
/// that solc emits:
/// - "abi"
/// - "evm.bytecode"
/// - "evm.deployedBytecode"
/// - "evm.methodIdentifiers"
/// that solc emits.
///
/// These correspond to the fields in `CompactBytecode`, `CompactDeployedBytecode`, ABI, and
/// method identfiers.
pub fn basic() -> Vec<Self> {
// We don't include all the `bytecode` fields because `generatedSources` is a massive JSON
// object and is not used by Foundry.
vec![
Self::Abi,
BytecodeOutputSelection::All.into(),
DeployedBytecodeOutputSelection::All.into(),
// The fields in `CompactBytecode`.
BytecodeOutputSelection::Object.into(),
BytecodeOutputSelection::SourceMap.into(),
BytecodeOutputSelection::LinkReferences.into(),
// The fields in `CompactDeployedBytecode`.
DeployedBytecodeOutputSelection::Object.into(),
DeployedBytecodeOutputSelection::SourceMap.into(),
DeployedBytecodeOutputSelection::LinkReferences.into(),
DeployedBytecodeOutputSelection::ImmutableReferences.into(),
EvmOutputSelection::MethodIdentifiers.into(),
]
}
Expand Down
4 changes: 2 additions & 2 deletions crates/artifacts/vyper/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct VyperInput {
}

impl VyperInput {
pub fn new(sources: Sources, mut settings: VyperSettings) -> Self {
pub fn new(sources: Sources, mut settings: VyperSettings, version: &Version) -> Self {
let mut new_sources = Sources::new();
let mut interfaces = Sources::new();

Expand All @@ -31,7 +31,7 @@ impl VyperInput {
}
}

settings.sanitize_output_selection();
settings.sanitize_output_selection(version);
Self { language: "Vyper".to_string(), sources: new_sources, interfaces, settings }
}

Expand Down
41 changes: 35 additions & 6 deletions crates/artifacts/vyper/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ use std::{
path::{Path, PathBuf},
};

pub const VYPER_SEARCH_PATHS: Version = Version::new(0, 4, 0);
pub const VYPER_SEARCH_PATHS: Version = VYPER_0_4;
pub const VYPER_BERLIN: Version = Version::new(0, 3, 0);
pub const VYPER_PARIS: Version = Version::new(0, 3, 7);
pub const VYPER_SHANGHAI: Version = Version::new(0, 3, 8);
pub const VYPER_CANCUN: Version = Version::new(0, 3, 8);

const VYPER_0_4: Version = Version::new(0, 4, 0);

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum VyperOptimizationMode {
Expand Down Expand Up @@ -65,15 +67,42 @@ impl VyperSettings {
});
}

/// During caching we prune output selection for some of the sources, however, Vyper will reject
/// [] as an output selection, so we are adding "abi" as a default output selection which is
/// cheap to be produced.
pub fn sanitize_output_selection(&mut self) {
/// Sanitize the output selection.
#[allow(clippy::collapsible_if)]
pub fn sanitize_output_selection(&mut self, version: &Version) {
self.output_selection.0.values_mut().for_each(|selection| {
selection.values_mut().for_each(|selection| {
// During caching we prune output selection for some of the sources, however, Vyper
// will reject `[]` as an output selection, so we are adding "abi" as a default
// output selection which is cheap to be produced.
if selection.is_empty() {
selection.push("abi".to_string())
}

// Unsupported selections.
#[rustfmt::skip]
selection.retain(|selection| {
if *version <= VYPER_0_4 {
if matches!(
selection.as_str(),
| "evm.bytecode.sourceMap" | "evm.deployedBytecode.sourceMap"
) {
return false;
}
}

if matches!(
selection.as_str(),
| "evm.bytecode.sourceMap" | "evm.deployedBytecode.sourceMap"
// https://github.com/vyperlang/vyper/issues/4389
| "evm.bytecode.linkReferences" | "evm.deployedBytecode.linkReferences"
| "evm.deployedBytecode.immutableReferences"
) {
return false;
}

true
});
})
});
}
Expand All @@ -84,7 +113,7 @@ impl VyperSettings {
self.search_paths = None;
}

self.sanitize_output_selection();
self.sanitize_output_selection(version);
self.normalize_evm_version(version);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/compilers/src/artifact_output/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ pub trait ArtifactOutput {

let mut files = contracts.keys().collect::<Vec<_>>();
// Iterate starting with top-most files to ensure that they get the shortest paths.
files.sort_by(|file1, file2| {
files.sort_by(|&file1, &file2| {
(file1.components().count(), file1).cmp(&(file2.components().count(), file2))
});
for file in files {
Expand Down
8 changes: 6 additions & 2 deletions crates/compilers/src/compilers/solc/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use semver::{Version, VersionReq};
use serde::{de::DeserializeOwned, Deserialize, Serialize};
use std::{
collections::BTreeSet,
io::{self, Write},
path::{Path, PathBuf},
process::{Command, Output, Stdio},
str::FromStr,
Expand Down Expand Up @@ -428,8 +429,11 @@ impl Solc {
let mut child = cmd.spawn().map_err(self.map_io_err())?;
debug!("spawned");

let stdin = child.stdin.as_mut().unwrap();
serde_json::to_writer(stdin, input)?;
{
let mut stdin = io::BufWriter::new(child.stdin.take().unwrap());
serde_json::to_writer(&mut stdin, input)?;
stdin.flush().map_err(self.map_io_err())?;
}
debug!("wrote JSON input to stdin");

let output = child.wait_with_output().map_err(self.map_io_err())?;
Expand Down
2 changes: 1 addition & 1 deletion crates/compilers/src/compilers/vyper/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl CompilerInput for VyperVersionedInput {
_language: Self::Language,
version: Version,
) -> Self {
Self { input: VyperInput::new(sources, settings), version }
Self { input: VyperInput::new(sources, settings, &version), version }
}

fn compiler_name(&self) -> Cow<'static, str> {
Expand Down
17 changes: 12 additions & 5 deletions crates/compilers/src/compilers/vyper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use foundry_compilers_core::error::{Result, SolcError};
use semver::Version;
use serde::{de::DeserializeOwned, Serialize};
use std::{
io::{self, Write},
path::{Path, PathBuf},
process::{Command, Stdio},
str::FromStr,
Expand Down Expand Up @@ -79,8 +80,11 @@ impl Vyper {

/// Convenience function for compiling all sources under the given path
pub fn compile_source(&self, path: &Path) -> Result<VyperOutput> {
let input =
VyperInput::new(Source::read_all_from(path, VYPER_EXTENSIONS)?, Default::default());
let input = VyperInput::new(
Source::read_all_from(path, VYPER_EXTENSIONS)?,
Default::default(),
&self.version,
);
self.compile(&input)
}

Expand Down Expand Up @@ -114,7 +118,7 @@ impl Vyper {
/// let vyper = Vyper::new("vyper")?;
/// let path = Path::new("path/to/sources");
/// let sources = Source::read_all_from(path, &["vy", "vyi"])?;
/// let input = VyperInput::new(sources, VyperSettings::default());
/// let input = VyperInput::new(sources, VyperSettings::default(), &vyper.version);
/// let output = vyper.compile(&input)?;
/// # Ok::<_, Box<dyn std::error::Error>>(())
/// ```
Expand Down Expand Up @@ -149,8 +153,11 @@ impl Vyper {
let mut child = cmd.spawn().map_err(self.map_io_err())?;
debug!("spawned");

let stdin = child.stdin.as_mut().unwrap();
serde_json::to_writer(stdin, input)?;
{
let mut stdin = io::BufWriter::new(child.stdin.take().unwrap());
serde_json::to_writer(&mut stdin, input)?;
stdin.flush().map_err(self.map_io_err())?;
}
debug!("wrote JSON input to stdin");

let output = child.wait_with_output().map_err(self.map_io_err())?;
Expand Down
43 changes: 33 additions & 10 deletions crates/compilers/tests/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,43 @@ pub static VYPER: LazyLock<Vyper> = LazyLock::new(|| {
#[cfg(target_family = "unix")]
use std::{fs::Permissions, os::unix::fs::PermissionsExt};

if let Ok(vyper) = Vyper::new("vyper") {
return vyper;
}

take_solc_installer_lock!(_lock);
let path = std::env::temp_dir().join("vyper");

if path.exists() {
return Vyper::new(&path).unwrap();
}

let url = match platform() {
Platform::MacOsAarch64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.darwin",
Platform::LinuxAmd64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.linux",
Platform::WindowsAmd64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.windows.exe",
_ => panic!("unsupported")
};

let res = reqwest::Client::builder().build().unwrap().get(url).send().await.unwrap();
let base = "https://github.com/vyperlang/vyper/releases/download/v0.4.0/vyper.0.4.0+commit.e9db8d9f";
let url = format!(
"{base}.{}",
match platform() {
Platform::MacOsAarch64 => "darwin",
Platform::LinuxAmd64 => "linux",
Platform::WindowsAmd64 => "windows.exe",
platform => panic!("unsupported platform: {platform:?}"),
}
);

assert!(res.status().is_success());
let mut retry = 3;
let mut res = None;
while retry > 0 {
match reqwest::get(&url).await.unwrap().error_for_status() {
Ok(res2) => {
res = Some(res2);
break;
}
Err(e) => {
eprintln!("{e}");
retry -= 1;
}
}
}
let res = res.expect("failed to get vyper binary");

let bytes = res.bytes().await.unwrap();

Expand Down Expand Up @@ -3889,17 +3909,20 @@ fn can_compile_vyper_with_cache() {
.unwrap();

let compiled = project.compile().unwrap();
compiled.assert_success();
assert!(compiled.find_first("Counter").is_some());
compiled.assert_success();

// cache is used when nothing to compile
let compiled = project.compile().unwrap();
compiled.assert_success();
assert!(compiled.find_first("Counter").is_some());
assert!(compiled.is_unchanged());

// deleted artifacts cause recompile even with cache
std::fs::remove_dir_all(project.artifacts_path()).unwrap();
let compiled = project.compile().unwrap();
compiled.assert_success();
assert!(compiled.find_first("Counter").is_some());
assert!(!compiled.is_unchanged());
}
Expand Down Expand Up @@ -3975,9 +3998,9 @@ fn test_can_compile_multi() {
.unwrap();

let compiled = project.compile().unwrap();
compiled.assert_success();
assert!(compiled.find(&root.join("src/Counter.sol"), "Counter").is_some());
assert!(compiled.find(&root.join("src/Counter.vy"), "Counter").is_some());
compiled.assert_success();
}

// This is a reproduction of https://github.com/foundry-rs/compilers/issues/47
Expand Down
14 changes: 0 additions & 14 deletions test-data/multi-sample/src/interfaces/ICounter.vy

This file was deleted.

11 changes: 11 additions & 0 deletions test-data/multi-sample/src/interfaces/ICounter.vyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# pragma version ^0.4.0

@external
@view
def number() -> uint256: ...

@external
def set_number(new_number: uint256): ...

@external
def increment() -> uint256: ...
2 changes: 0 additions & 2 deletions test-data/vyper-imports/src/A.vy
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
from . import B
import A as AAlias
from .module import C
1 change: 0 additions & 1 deletion test-data/vyper-imports/src/module/inner/E.vy
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
from ...module.inner import E
Loading

0 comments on commit 2199839

Please sign in to comment.