Skip to content

Commit

Permalink
Reuse single shared allocation for ABI data (#970)
Browse files Browse the repository at this point in the history
Currently whenever you create a new instance of a contract the entire
`Abi` gets cloned (which can be really huge) plus a bunch of signatures
get computed (a lot of hashing) and cached. This is really costly
considering that the `Abi` of a contract never changes so it would be an
easy win to share the same allocation of those immutable pieces of data
(`Abi` and computed hashes) across all instances of the same contract.
This would make the creation of subsequent instances of the same
contract mostly free.

This can already be achieved with ugly workarounds discussed and
implemented [here](cowprotocol/services#2628)
but if this issue gets resolved in the library itself manual allocation
optimization will not be needed and the performance improvement for any
code that creates a lot of contract instances would be huge.

The actual change is overall pretty small. I created a new type
`Interface` that contains all the immutable data derived from the `Abi`.
Then I gave it custom `(De)Serialize` implementations which mostly just
do the same thing it did before (defer to `Abi`'s implementation). The
`Deserialize` implementation of course also computes the important hash
and stores it's values together with the `Abi`.
Because the typesafe rust binding code generated by `ethcontract-rs`
already stores the raw contract in a static allocation (which now
contains the relevant immutable state `Arc`ed) we now get cheap contract
instantiations.

Thanks @nlordell for the idea of fixing the performance issue directly
in the library. 👍

The code also has a few unrelated changes to make the `nightly` CI build
pass.

### Breaking changes
* `Artifact::insert` can now run into annoying borrow checker issues
(that can be worked around, though)
* `Contract:abi` is now accessible with `Contract::interface::abi`
* `Contract::interface::abi` is harder to mutate than `Contract::abi`
due to the introduction of the `Arc`

Overall I don't know how many people actually rely on this crate and
whether or not these breaking changes would be a big deal for them. But
even if this should not get merged into the main release of the crate I
think this patch is still useful for anybody who needs to optimize
performance across the board.

### Test Plan
existing tests
ran cow protocol backend using the patched library and didn't see any
obvious issues
  • Loading branch information
MartinquaXD committed Apr 22, 2024
1 parent db4a20f commit b75f193
Show file tree
Hide file tree
Showing 21 changed files with 219 additions and 133 deletions.
4 changes: 2 additions & 2 deletions ethcontract-common/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ethcontract-common"
version = "0.25.5"
version = "0.25.6"
authors = ["Gnosis developers <developers@gnosis.io>"]
edition = "2021"
license = "MIT OR Apache-2.0"
Expand All @@ -14,7 +14,7 @@ Common types for ethcontract-rs runtime and proc macro.
[dependencies]
ethabi = "18.0"
hex = "0.4"
serde = "1.0"
serde= { version = "1.0", features = ["rc"] }
serde_derive = "1.0"
serde_json = "1.0"
thiserror = "1.0"
Expand Down
41 changes: 30 additions & 11 deletions ethcontract-common/src/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
//! artifact models. It also provides tools to load artifacts from different
//! sources, and parse them using different formats.

use crate::contract::{Documentation, Network};
use crate::contract::{Documentation, Interface, Network};
use crate::{Abi, Bytecode, Contract};
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::ops::Deref;
use std::sync::Arc;

pub mod hardhat;
pub mod truffle;
Expand Down Expand Up @@ -151,7 +152,7 @@ pub struct ContractMut<'a>(&'a mut Contract);
impl<'a> ContractMut<'a> {
/// Returns mutable reference to contract's abi.
pub fn abi_mut(&mut self) -> &mut Abi {
&mut self.0.abi
&mut Arc::make_mut(&mut self.0.interface).abi
}

/// Returns mutable reference to contract's bytecode.
Expand Down Expand Up @@ -188,6 +189,18 @@ impl Deref for ContractMut<'_> {
}
}

impl Drop for ContractMut<'_> {
fn drop(&mut self) {
// The ABI might have gotten mutated while this guard was alive.
// Since we compute pre-compute and cache a few values based on the ABI
// as a performance optimization we need to recompute those cached values
// with the new ABI once the user is done updating the mutable contract.
let abi = self.0.interface.abi.clone();
let interface = Interface::from(abi);
*Arc::make_mut(&mut self.0.interface) = interface;
}
}

#[cfg(test)]
mod test {
use super::*;
Expand All @@ -204,26 +217,32 @@ mod test {

assert_eq!(artifact.len(), 0);

let insert_res = artifact.insert(make_contract("C1"));
{
let insert_res = artifact.insert(make_contract("C1"));

assert_eq!(insert_res.inserted_contract.name, "C1");
assert!(insert_res.old_contract.is_none());
assert_eq!(insert_res.inserted_contract.name, "C1");
assert!(insert_res.old_contract.is_none());
}

assert_eq!(artifact.len(), 1);
assert!(artifact.contains("C1"));

let insert_res = artifact.insert(make_contract("C2"));
{
let insert_res = artifact.insert(make_contract("C2"));

assert_eq!(insert_res.inserted_contract.name, "C2");
assert!(insert_res.old_contract.is_none());
assert_eq!(insert_res.inserted_contract.name, "C2");
assert!(insert_res.old_contract.is_none());
}

assert_eq!(artifact.len(), 2);
assert!(artifact.contains("C2"));

let insert_res = artifact.insert(make_contract("C1"));
{
let insert_res = artifact.insert(make_contract("C1"));

assert_eq!(insert_res.inserted_contract.name, "C1");
assert!(insert_res.old_contract.is_some());
assert_eq!(insert_res.inserted_contract.name, "C1");
assert!(insert_res.old_contract.is_some());
}

assert_eq!(artifact.len(), 2);
}
Expand Down
19 changes: 11 additions & 8 deletions ethcontract-common/src/artifact/hardhat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,15 +426,18 @@ impl HardHatLoader {
address: Address,
transaction_hash: Option<TransactionHash>,
) -> Result<(), ArtifactError> {
let mut contract = match artifact.get_mut(&contract.name) {
Some(existing_contract) => {
if existing_contract.abi != contract.abi {
return Err(ArtifactError::AbiMismatch(contract.name));
}

existing_contract
let contract_guard = artifact.get_mut(&contract.name);
let mut contract = if let Some(existing_contract) = contract_guard {
if existing_contract.interface != contract.interface {
return Err(ArtifactError::AbiMismatch(contract.name));
}
None => artifact.insert(contract).inserted_contract,

existing_contract
} else {
// `Drop` of the contract guard can update the underlying contract which can lead
// to borrowing issues. To work around those we manually drop the guard here.
drop(contract_guard);
artifact.insert(contract).inserted_contract
};

let deployment_information = transaction_hash.map(DeploymentInformation::TransactionHash);
Expand Down
2 changes: 1 addition & 1 deletion ethcontract-common/src/artifact/truffle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl TruffleLoader {
let mut contract: Contract = loader(source)?;

if let Some(name) = &self.name {
contract.name = name.clone();
contract.name.clone_from(name);
}

Ok(contract)
Expand Down
81 changes: 77 additions & 4 deletions ethcontract-common/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
//! Module for reading and examining data produced by truffle.

use crate::abiext::FunctionExt;
use crate::hash::H32;
use crate::Abi;
use crate::{bytecode::Bytecode, DeploymentInformation};
use ethabi::ethereum_types::H256;
use serde::Deserializer;
use serde::Serializer;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::hash::Hash;
use std::sync::Arc;
use web3::types::Address;

/// Represents a contract data.
Expand All @@ -13,8 +20,9 @@ pub struct Contract {
/// The contract name. Unnamed contracts have an empty string as their name.
#[serde(rename = "contractName")]
pub name: String,
/// The contract ABI
pub abi: Abi,
/// The contract interface.
#[serde(rename = "abi")]
pub interface: Arc<Interface>,
/// The contract deployment bytecode.
pub bytecode: Bytecode,
/// The contract's expected deployed bytecode.
Expand All @@ -28,6 +36,71 @@ pub struct Contract {
pub userdoc: Documentation,
}

/// Struct representing publicly accessible interface of a smart contract.
#[derive(Clone, Debug, Default, PartialEq)]
pub struct Interface {
/// The contract ABI
pub abi: Abi,
/// A mapping from method signature to a name-index pair for accessing
/// functions in the contract ABI. This is used to avoid allocation when
/// searching for matching functions by signature.
pub methods: HashMap<H32, (String, usize)>,
/// A mapping from event signature to a name-index pair for resolving
/// events in the contract ABI.
pub events: HashMap<H256, (String, usize)>,
}

impl<'de> Deserialize<'de> for Interface {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let abi = Abi::deserialize(deserializer)?;
Ok(abi.into())
}
}

impl Serialize for Interface {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
self.abi.serialize(serializer)
}
}

impl From<Abi> for Interface {
fn from(abi: Abi) -> Self {
Self {
methods: create_mapping(&abi.functions, |function| function.selector()),
events: create_mapping(&abi.events, |event| event.signature()),
abi,
}
}
}

/// Utility function for creating a mapping between a unique signature and a
/// name-index pair for accessing contract ABI items.
fn create_mapping<T, S, F>(
elements: &BTreeMap<String, Vec<T>>,
signature: F,
) -> HashMap<S, (String, usize)>
where
S: Hash + Eq + Ord,
F: Fn(&T) -> S,
{
let signature = &signature;
elements
.iter()
.flat_map(|(name, sub_elements)| {
sub_elements
.iter()
.enumerate()
.map(move |(index, element)| (signature(element), (name.to_owned(), index)))
})
.collect()
}

impl Contract {
/// Creates an empty contract instance.
pub fn empty() -> Self {
Expand All @@ -38,7 +111,7 @@ impl Contract {
pub fn with_name(name: impl Into<String>) -> Self {
Contract {
name: name.into(),
abi: Default::default(),
interface: Default::default(),
bytecode: Default::default(),
deployed_bytecode: Default::default(),
networks: HashMap::new(),
Expand Down
6 changes: 3 additions & 3 deletions ethcontract-derive/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ethcontract-derive"
version = "0.25.5"
version = "0.25.6"
authors = ["Gnosis developers <developers@gnosis.io>"]
edition = "2021"
license = "MIT OR Apache-2.0"
Expand All @@ -20,8 +20,8 @@ proc-macro = true

[dependencies]
anyhow = "1.0"
ethcontract-common = { version = "0.25.5", path = "../ethcontract-common" }
ethcontract-generate = { version = "0.25.5", path = "../ethcontract-generate", default-features = false }
ethcontract-common = { version = "0.25.6", path = "../ethcontract-common" }
ethcontract-generate = { version = "0.25.6", path = "../ethcontract-generate", default-features = false }
proc-macro2 = "1.0"
quote = "1.0"
syn = "2.0"
4 changes: 2 additions & 2 deletions ethcontract-generate/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ethcontract-generate"
version = "0.25.5"
version = "0.25.6"
authors = ["Gnosis developers <developers@gnosis.io>"]
edition = "2021"
license = "MIT OR Apache-2.0"
Expand All @@ -18,7 +18,7 @@ http = ["curl"]
[dependencies]
anyhow = "1.0"
curl = { version = "0.4", optional = true }
ethcontract-common = { version = "0.25.5", path = "../ethcontract-common" }
ethcontract-common = { version = "0.25.6", path = "../ethcontract-common" }
Inflector = "0.11"
proc-macro2 = "1.0"
quote = "1.0"
Expand Down
6 changes: 3 additions & 3 deletions ethcontract-generate/src/generate/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) fn expand(cx: &Context) -> TokenStream {
lazy_static! {
pub static ref CONTRACT: Contract = {
#[allow(unused_mut)]
let mut contract = TruffleLoader::new()
let mut contract: Contract = TruffleLoader::new()
.load_contract_from_str(#contract_json)
.expect("valid contract JSON");
#( #deployments )*
Expand Down Expand Up @@ -146,8 +146,8 @@ pub(crate) fn expand(cx: &Context) -> TokenStream {

let transport = DynTransport::new(web3.transport().clone());
let web3 = Web3::new(transport);
let abi = Self::raw_contract().abi.clone();
let instance = Instance::with_deployment_info(web3, abi, address, deployment_information);
let interface = Self::raw_contract().interface.clone();
let instance = Instance::with_deployment_info(web3, interface, address, deployment_information);

Contract::from_raw(instance)
}
Expand Down
4 changes: 2 additions & 2 deletions ethcontract-generate/src/generate/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn expand_deploy(cx: &Context) -> Result<TokenStream> {
// can't seem to get truffle to output it
let doc = util::expand_doc("Generated by `ethcontract`");

let (input, arg) = match cx.contract.abi.constructor() {
let (input, arg) = match cx.contract.interface.abi.constructor() {
Some(constructor) => (
methods::expand_inputs(&constructor.inputs)?,
methods::expand_inputs_call_arg(&constructor.inputs),
Expand Down Expand Up @@ -190,7 +190,7 @@ fn expand_deploy(cx: &Context) -> Result<TokenStream> {
}

fn abi(_: &Self::Context) -> &self::ethcontract::common::Abi {
&Self::raw_contract().abi
&Self::raw_contract().interface.abi
}

fn from_deployment(
Expand Down

0 comments on commit b75f193

Please sign in to comment.