Skip to content

Commit

Permalink
feat: Restore hashing args via slice for performance (AztecProtocol/a…
Browse files Browse the repository at this point in the history
…ztec-packages#5539)

I replaced the ArgsHasher struct with a BoundedVec because a noir
version completely separated slices and generic arrays in the frontend,
so we couldn't pass it to the hash_args fn that needed an array. This
makes unconstrained fns slower since they have to allocate in memory the
full max length that the arguments could have.
In this PR I instead move to a full slice approach, where hash_args
takes a slice and hash_args_array just casts it to a slice. This avoids
allocating memory unnecessarily in public functions.
  • Loading branch information
AztecBot committed Apr 3, 2024
1 parent a0f7474 commit 2179e51
Show file tree
Hide file tree
Showing 55 changed files with 493 additions and 1,677 deletions.
2 changes: 1 addition & 1 deletion .aztec-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
bb719200034e3bc6db09fb56538dadca4203abf4
eb3acdf16d58968ac67ffad4c2d5efb10fa31d26
3 changes: 1 addition & 2 deletions .github/Cross.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
passthrough = [
"HOME",
"RUST_BACKTRACE",
"BARRETENBERG_BIN_DIR",
"BLNS_JSON_PATH"
"BARRETENBERG_BIN_DIR"
]
volumes = [
"HOME",
Expand Down
14 changes: 1 addition & 13 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,6 @@ jobs:
- name: Install Yarn dependencies
uses: ./.github/actions/setup

- name: Install wasm-bindgen-cli
uses: taiki-e/install-action@v2
with:
tool: wasm-bindgen-cli@0.2.86

- name: Install wasm-opt
run: |
npm i wasm-opt -g
- name: Query new noir version
id: noir-version
run: |
Expand Down Expand Up @@ -116,20 +107,17 @@ jobs:
if: ${{ always() }}

needs:
- release-please
- update-acvm-workspace-package-versions
- update-docs

env:
# We treat any skipped or failing jobs as a failure for the workflow as a whole.
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }

steps:
- name: Add warning to sticky comment
uses: marocchino/sticky-pull-request-comment@v2
with:
# We need to specify the PR on which to make the comment as workflow is triggered by push.
number: ${{ fromJSON(needs.release-please.outputs.release-pr).number }}
# delete the comment in case failures have been fixed
delete: ${{ !env.FAIL }}
message: "The release workflow has not completed successfully. Releasing now will result in a broken release"
Expand Down
16 changes: 7 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

126 changes: 68 additions & 58 deletions aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,62 +223,66 @@ fn create_assert_initializer() -> Statement {
/// ```noir
/// #[aztec(private)]
/// fn foo(structInput: SomeStruct, arrayInput: [u8; 10], fieldInput: Field) -> Field {
/// // Create the bounded vec object
/// let mut serialized_args = BoundedVec::new();
/// // Create the hasher object
/// let mut hasher = Hasher::new();
///
/// // struct inputs call serialize on them to add an array of fields
/// serialized_args.extend_from_array(structInput.serialize());
/// hasher.add_multiple(structInput.serialize());
///
/// // Array inputs are iterated over and each element is added to the bounded vec (as a field)
/// // Array inputs are iterated over and each element is added to the hasher (as a field)
/// for i in 0..arrayInput.len() {
/// serialized_args.push(arrayInput[i] as Field);
/// hasher.add(arrayInput[i] as Field);
/// }
/// // Field inputs are added to the bounded vec
/// serialized_args.push({ident});
/// // Field inputs are added to the hasher
/// hasher.add({ident});
///
/// // Create the context
/// // The inputs (injected by this `create_inputs`) and completed hash object are passed to the context
/// let mut context = PrivateContext::new(inputs, hash_args(serialized_args));
/// let mut context = PrivateContext::new(inputs, hasher.hash());
/// }
/// ```
fn create_context(ty: &str, params: &[Param]) -> Result<Vec<Statement>, AztecMacroError> {
let mut injected_expressions: Vec<Statement> = vec![];

// `let mut serialized_args = BoundedVec::new();`
let let_serialized_args = mutable_assignment(
"serialized_args", // Assigned to
let hasher_name = "args_hasher";

// `let mut args_hasher = Hasher::new();`
let let_hasher = mutable_assignment(
hasher_name, // Assigned to
call(
variable_path(chained_dep!("std", "collections", "bounded_vec", "BoundedVec", "new")), // Path
vec![], // args
variable_path(chained_dep!("aztec", "hash", "ArgsHasher", "new")), // Path
vec![], // args
),
);

// Completes: `let mut serialized_args = BoundedVec::new();`
injected_expressions.push(let_serialized_args);
// Completes: `let mut args_hasher = Hasher::new();`
injected_expressions.push(let_hasher);

// Iterate over each of the function parameters, adding to them to the bounded vec
// Iterate over each of the function parameters, adding to them to the hasher
for Param { pattern, typ, span, .. } in params {
match pattern {
Pattern::Identifier(identifier) => {
// Match the type to determine the padding to do
let unresolved_type = &typ.typ;
let expression = match unresolved_type {
// `serialized_args.extend_from_array({ident}.serialize())`
UnresolvedTypeData::Named(..) => add_struct_to_serialized_args(identifier),
// `hasher.add_multiple({ident}.serialize())`
UnresolvedTypeData::Named(..) => add_struct_to_hasher(identifier, hasher_name),
UnresolvedTypeData::Array(_, arr_type) => {
add_array_to_serialized_args(identifier, arr_type)
add_array_to_hasher(identifier, arr_type, hasher_name)
}
// `hasher.add({ident})`
UnresolvedTypeData::FieldElement => {
add_field_to_hasher(identifier, hasher_name)
}
// `serialized_args.push({ident})`
UnresolvedTypeData::FieldElement => add_field_to_serialized_args(identifier),
// Add the integer to the serialized args, casted to a field
// `serialized_args.push({ident} as Field)`
// Add the integer to the hasher, casted to a field
// `hasher.add({ident} as Field)`
UnresolvedTypeData::Integer(..) | UnresolvedTypeData::Bool => {
add_cast_to_serialized_args(identifier)
add_cast_to_hasher(identifier, hasher_name)
}
UnresolvedTypeData::String(..) => {
let (var_bytes, id) = str_to_bytes(identifier);
injected_expressions.push(var_bytes);
add_array_to_serialized_args(
add_array_to_hasher(
&id,
&UnresolvedType {
typ: UnresolvedTypeData::Integer(
Expand All @@ -287,6 +291,7 @@ fn create_context(ty: &str, params: &[Param]) -> Result<Vec<Statement>, AztecMac
),
span: None,
},
hasher_name,
)
}
_ => {
Expand All @@ -304,10 +309,11 @@ fn create_context(ty: &str, params: &[Param]) -> Result<Vec<Statement>, AztecMac

// Create the inputs to the context
let inputs_expression = variable("inputs");
// `hash_args(serialized_args)`
let hash_call = call(
variable_path(chained_dep!("aztec", "hash", "hash_args")), // variable
vec![variable("serialized_args")], // args
// `args_hasher.hash()`
let hash_call = method_call(
variable(hasher_name), // variable
"hash", // method name
vec![], // args
);

let path_snippet = ty.to_case(Case::Snake); // e.g. private_context
Expand Down Expand Up @@ -591,21 +597,21 @@ fn create_context_finish() -> Statement {
}

//
// Methods to create hash_args inputs
// Methods to create hasher inputs
//

fn add_struct_to_serialized_args(identifier: &Ident) -> Statement {
// If this is a struct, we call serialize and add the array to the serialized args
fn add_struct_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement {
// If this is a struct, we call serialize and add the array to the hasher
let serialized_call = method_call(
variable_path(path(identifier.clone())), // variable
"serialize", // method name
vec![], // args
);

make_statement(StatementKind::Semi(method_call(
variable("serialized_args"), // variable
"extend_from_array", // method name
vec![serialized_call], // args
variable(hasher_name), // variable
"add_multiple", // method name
vec![serialized_call], // args
)))
}

Expand All @@ -625,7 +631,7 @@ fn str_to_bytes(identifier: &Ident) -> (Statement, Ident) {
}

fn create_loop_over(var: Expression, loop_body: Vec<Statement>) -> Statement {
// If this is an array of primitive types (integers / fields) we can add them each to the serialized args
// If this is an array of primitive types (integers / fields) we can add them each to the hasher
// casted to a field
let span = var.span;

Expand All @@ -638,7 +644,7 @@ fn create_loop_over(var: Expression, loop_body: Vec<Statement>) -> Statement {

// What will be looped over

// - `serialized_args.push({ident}[i] as Field)`
// - `hasher.add({ident}[i] as Field)`
let for_loop_block =
expression(ExpressionKind::Block(BlockExpression { statements: loop_body }));

Expand All @@ -657,66 +663,70 @@ fn create_loop_over(var: Expression, loop_body: Vec<Statement>) -> Statement {
}))
}

fn add_array_to_serialized_args(identifier: &Ident, arr_type: &UnresolvedType) -> Statement {
// If this is an array of primitive types (integers / fields) we can add them each to the serialized_args
fn add_array_to_hasher(
identifier: &Ident,
arr_type: &UnresolvedType,
hasher_name: &str,
) -> Statement {
// If this is an array of primitive types (integers / fields) we can add them each to the hasher
// casted to a field

// Wrap in the semi thing - does that mean ended with semi colon?
// `serialized_args.push({ident}[i] as Field)`
// `hasher.add({ident}[i] as Field)`

let arr_index = index_array(identifier.clone(), "i");
let (add_expression, vec_method_name) = match arr_type.typ {
let (add_expression, hasher_method_name) = match arr_type.typ {
UnresolvedTypeData::Named(..) => {
let vec_method_name = "extend_from_array".to_owned();
let hasher_method_name = "add_multiple".to_owned();
let call = method_call(
// All serialize on each element
arr_index, // variable
"serialize", // method name
vec![], // args
);
(call, vec_method_name)
(call, hasher_method_name)
}
_ => {
let vec_method_name = "push".to_owned();
let hasher_method_name = "add".to_owned();
let call = cast(
arr_index, // lhs - `ident[i]`
UnresolvedTypeData::FieldElement, // cast to - `as Field`
);
(call, vec_method_name)
(call, hasher_method_name)
}
};

let block_statement = make_statement(StatementKind::Semi(method_call(
variable("serialized_args"), // variable
&vec_method_name, // method name
variable(hasher_name), // variable
&hasher_method_name, // method name
vec![add_expression],
)));

create_loop_over(variable_ident(identifier.clone()), vec![block_statement])
}

fn add_field_to_serialized_args(identifier: &Ident) -> Statement {
// `serialized_args.push({ident})`
fn add_field_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement {
// `hasher.add({ident})`
let ident = variable_path(path(identifier.clone()));
make_statement(StatementKind::Semi(method_call(
variable("serialized_args"), // variable
"push", // method name
vec![ident], // args
variable(hasher_name), // variable
"add", // method name
vec![ident], // args
)))
}

fn add_cast_to_serialized_args(identifier: &Ident) -> Statement {
// `serialized_args.push({ident} as Field)`
fn add_cast_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement {
// `hasher.add({ident} as Field)`
// `{ident} as Field`
let cast_operation = cast(
variable_path(path(identifier.clone())), // lhs
UnresolvedTypeData::FieldElement, // rhs
);

// `serialized_args.push({ident} as Field)`
// `hasher.add({ident} as Field)`
make_statement(StatementKind::Semi(method_call(
variable("serialized_args"), // variable
"push", // method name
vec![cast_operation], // args
variable(hasher_name), // variable
"add", // method name
vec![cast_operation], // args
)))
}
Loading

0 comments on commit 2179e51

Please sign in to comment.