From 56f2bd0cbe0629a349f98f7a30f8d5f46c056e28 Mon Sep 17 00:00:00 2001 From: codygunton Date: Sun, 29 Sep 2024 04:44:04 +0000 Subject: [PATCH 1/6] Do it. Results: x86 Benchmark Time CPU Time Old Time New CPU Old CPU New -------------------------------------------------------------------------------------------------------------------- ClientIVCBench/Full/6 -0.0072 -0.0144 33274 33036 30863 30419 OVERALL_GEOMEAN -0.0072 -0.0144 33 33 31 30 WASM Benchmark Time CPU Time Old Time New CPU Old CPU New -------------------------------------------------------------------------------------------------------------------- ClientIVCBench/Full/6 +0.0024 +0.0024 102372 102619 102371747000 102619375000 OVERALL_GEOMEAN +0.0024 +0.0024 102 103 102371747 102619375 --- .../protogalaxy_prover_internal.hpp | 68 +++++++++++++------ .../cpp/src/barretenberg/relations/utils.hpp | 13 ++-- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp index 4fd90abed34..b50a6bba087 100644 --- a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp +++ b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp @@ -24,6 +24,7 @@ template class ProtogalaxyProverInternal { using RelationUtils = bb::RelationUtils; using ProverPolynomials = typename Flavor::ProverPolynomials; using Relations = typename Flavor::Relations; + using AllValues = typename Flavor::AllValues; using RelationSeparator = typename Flavor::RelationSeparator; static constexpr size_t NUM_KEYS = DeciderProvingKeys_::NUM; using UnivariateRelationParametersNoOptimisticSkipping = @@ -54,6 +55,28 @@ template class ProtogalaxyProverInternal { static constexpr size_t NUM_SUBRELATIONS = DeciderPKs::NUM_SUBRELATIONS; + static FF process_subrelation_evaluations(const RelationEvaluations& evals, + const std::array& challenges, + FF& linearly_dependent_contribution) + { + FF linearly_independent_contribution{ 0 }; + size_t idx = 0; + + auto scale_by_challenge_and_accumulate = + [&](Element& element) { + using Relation = typename std::tuple_element_t; + const Element contribution = element * challenges[idx]; + if (subrelation_is_linearly_independent()) { + linearly_independent_contribution += contribution; + } else { + linearly_dependent_contribution += contribution; + } + idx++; + }; + RelationUtils::apply_to_tuple_of_arrays_elements(scale_by_challenge_and_accumulate, evals); + return linearly_independent_contribution; + } + /** * @brief Compute the values of the aggregated relation evaluations at each row in the execution trace, representing * f_i(ω) in the Protogalaxy paper, given the evaluations of all the prover polynomials and \vec{α} (the batching @@ -67,40 +90,41 @@ template class ProtogalaxyProverInternal { * linearly dependent subrelation and α_j is its corresponding batching challenge. */ static std::vector compute_row_evaluations(const ProverPolynomials& polynomials, - const RelationSeparator& alpha, + const RelationSeparator& alphas_, const RelationParameters& relation_parameters) { BB_OP_COUNT_TIME_NAME("ProtogalaxyProver_::compute_row_evaluations"); + const size_t polynomial_size = polynomials.get_polynomial_size(); - std::vector full_honk_evaluations(polynomial_size); + std::vector aggregated_relation_evaluations(polynomial_size); + + const std::array alphas = [&alphas_]() { + std::array tmp; + tmp[0] = 1; + std::copy(alphas_.begin(), alphas_.end(), tmp.begin() + 1); + return tmp; + }(); + const std::vector linearly_dependent_contribution_accumulators = parallel_for_heuristic( polynomial_size, /*accumulator default*/ FF(0), - [&](size_t row, FF& linearly_dependent_contribution_accumulator) { - auto row_evaluations = polynomials.get_row(row); - RelationEvaluations relation_evaluations; - RelationUtils::zero_elements(relation_evaluations); - - RelationUtils::template accumulate_relation_evaluations<>( - row_evaluations, relation_evaluations, relation_parameters, FF(1)); - - auto output = FF(0); - auto running_challenge = FF(1); - RelationUtils::scale_and_batch_elements(relation_evaluations, - alpha, - running_challenge, - output, - linearly_dependent_contribution_accumulator); - - full_honk_evaluations[row] = output; + [&](size_t row_idx, FF& linearly_dependent_contribution_accumulator) { + const AllValues row = polynomials.get_row(row_idx); + // Evaluate all subrelations on the given row. Separator is 1 since we are not summing across rows here. + const RelationEvaluations evals = + RelationUtils::accumulate_relation_evaluations(row, relation_parameters, FF(1)); + + // Sum against challenges alpha + aggregated_relation_evaluations[row_idx] = + process_subrelation_evaluations(evals, alphas, linearly_dependent_contribution_accumulator); }, thread_heuristics::ALWAYS_MULTITHREAD); - full_honk_evaluations[0] += sum(linearly_dependent_contribution_accumulators); - return full_honk_evaluations; - } + aggregated_relation_evaluations[0] += sum(linearly_dependent_contribution_accumulators); + return aggregated_relation_evaluations; + } /** * @brief Recursively compute the parent nodes of each level in the tree, starting from the leaves. Note that at * each level, the resulting parent nodes will be polynomials of degree (level+1) because we multiply by an diff --git a/barretenberg/cpp/src/barretenberg/relations/utils.hpp b/barretenberg/cpp/src/barretenberg/relations/utils.hpp index 0d3ae711ba1..b4d6e35a3df 100644 --- a/barretenberg/cpp/src/barretenberg/relations/utils.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/utils.hpp @@ -174,15 +174,16 @@ template class RelationUtils { */ template // TODO(#224)(Cody): Input should be an array? - inline static void accumulate_relation_evaluations(const PolynomialEvaluations& evaluations, - RelationEvaluations& relation_evaluations, - const Parameters& relation_parameters, - const FF& partial_evaluation_result) + inline static RelationEvaluations accumulate_relation_evaluations(const PolynomialEvaluations& evaluations, + const Parameters& relation_parameters, + const FF& partial_evaluation_result) { + RelationEvaluations result; constexpr_for<0, NUM_RELATIONS, 1>([&]() { accumulate_single_relation( - evaluations, relation_evaluations, relation_parameters, partial_evaluation_result); + evaluations, result, relation_parameters, partial_evaluation_result); }); + return result; } template @@ -336,7 +337,7 @@ template class RelationUtils { * dependent contribution when we compute the evaluation of full rel_U(G)H at particular row.) */ template - static void apply_to_tuple_of_arrays_elements(Operation&& operation, std::tuple& tuple) + static void apply_to_tuple_of_arrays_elements(Operation&& operation, const std::tuple& tuple) { using Relation = typename std::tuple_element_t; const auto subrelation_length = Relation::SUBRELATION_PARTIAL_LENGTHS.size(); From 4514ac2190473c83d9bb9295ed248b1599d4f86b Mon Sep 17 00:00:00 2001 From: codygunton Date: Sun, 29 Sep 2024 15:31:29 +0000 Subject: [PATCH 2/6] Error handling in compare scripts. --- .../cpp/scripts/compare_branch_vs_baseline_remote.sh | 5 +++++ .../cpp/scripts/compare_branch_vs_baseline_remote_wasm.sh | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote.sh b/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote.sh index edd23d05119..7c9192fa3f7 100755 --- a/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote.sh +++ b/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote.sh @@ -19,6 +19,11 @@ HARDWARE_CONCURRENCY=${HARDWARE_CONCURRENCY:-16} BASELINE_BRANCH="master" BENCH_TOOLS_DIR="$BUILD_DIR/_deps/benchmark-src/tools" +if [ ! -z "$(git status --untracked-files=no --porcelain)" ]; then + echo "Git status is unclean; the script will not be able to check out $BASELINE_BRANCH." + exit 1 +fi + echo -e "\nComparing $BENCHMARK between $BASELINE_BRANCH and current branch:" # Move above script dir. diff --git a/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote_wasm.sh b/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote_wasm.sh index d7732ffc41a..aaee9d0f3b6 100755 --- a/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote_wasm.sh +++ b/barretenberg/cpp/scripts/compare_branch_vs_baseline_remote_wasm.sh @@ -19,6 +19,12 @@ HARDWARE_CONCURRENCY=${HARDWARE_CONCURRENCY:-16} BASELINE_BRANCH="master" BENCH_TOOLS_DIR="$BUILD_DIR/_deps/benchmark-src/tools" +if [ ! -z "$(git status --untracked-files=no --porcelain)" ]; then + echo "Git status is unclean; the script will not be able to check out $BASELINE_BRANCH." + exit 1 +fi + + echo -e "\nComparing $BENCHMARK between $BASELINE_BRANCH and current branch:" # Move above script dir. From 6035a511c9734fb752b77ebe6f3301f67a6a3d7b Mon Sep 17 00:00:00 2001 From: codygunton Date: Sun, 29 Sep 2024 15:31:39 +0000 Subject: [PATCH 3/6] Inline process function --- .../protogalaxy/protogalaxy_prover_internal.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp index b50a6bba087..53231d3c7e0 100644 --- a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp +++ b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp @@ -55,9 +55,9 @@ template class ProtogalaxyProverInternal { static constexpr size_t NUM_SUBRELATIONS = DeciderPKs::NUM_SUBRELATIONS; - static FF process_subrelation_evaluations(const RelationEvaluations& evals, - const std::array& challenges, - FF& linearly_dependent_contribution) + inline static FF process_subrelation_evaluations(const RelationEvaluations& evals, + const std::array& challenges, + FF& linearly_dependent_contribution) { FF linearly_independent_contribution{ 0 }; size_t idx = 0; From 42deea0ed3401db966c3f47033dd5e217be7c2cc Mon Sep 17 00:00:00 2001 From: codygunton Date: Sun, 29 Sep 2024 15:45:31 +0000 Subject: [PATCH 4/6] Add compare script for x86 --- barretenberg/cpp/scripts/compare_client_ivc_bench.sh | 2 +- barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100755 barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh diff --git a/barretenberg/cpp/scripts/compare_client_ivc_bench.sh b/barretenberg/cpp/scripts/compare_client_ivc_bench.sh index aa4179d6df0..01c6f367b0c 100755 --- a/barretenberg/cpp/scripts/compare_client_ivc_bench.sh +++ b/barretenberg/cpp/scripts/compare_client_ivc_bench.sh @@ -1,4 +1,4 @@ #!/usr/bin/env bash set -eu -./scripts/compare_branch_vs_baseline_remote_wasm.sh client_ivc_bench 'Full/6$' \ No newline at end of file +./scripts/compare_branch_vs_baseline_remote.sh client_ivc_bench 'Full/6$' \ No newline at end of file diff --git a/barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh b/barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh new file mode 100755 index 00000000000..aa4179d6df0 --- /dev/null +++ b/barretenberg/cpp/scripts/compare_client_ivc_bench_wasm.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash +set -eu + +./scripts/compare_branch_vs_baseline_remote_wasm.sh client_ivc_bench 'Full/6$' \ No newline at end of file From 9464074ac7d13bb828e9ad38af9f18960150f695 Mon Sep 17 00:00:00 2001 From: codygunton Date: Sun, 29 Sep 2024 16:06:43 +0000 Subject: [PATCH 5/6] Remove deprecated function and add docs for replacement --- .../protogalaxy_prover_internal.hpp | 13 ++++++ .../cpp/src/barretenberg/relations/utils.hpp | 42 ------------------- 2 files changed, 13 insertions(+), 42 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp index 53231d3c7e0..1bdb3ff6201 100644 --- a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp +++ b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp @@ -55,6 +55,19 @@ template class ProtogalaxyProverInternal { static constexpr size_t NUM_SUBRELATIONS = DeciderPKs::NUM_SUBRELATIONS; + /** + * @brief A scale subrelations evaluations by challenges ('alphas') and part of the linearly dependent relation + * evaluation(s). + * + * @details Note that a linearly dependent subrelation is not computed on a specific row but rather on the entire + * execution trace. + * + * @param evals The evaluations of all subrelations on some row + * @param challenges The 'alpha' challenges used to batch the subrelations + * @param linearly_dependent_contribution An accumulator for values of the linearly-dependent (i.e., 'whole-trace') + * subrelations + * @return FF The evaluation of the linearly-independent (i.e., 'per-row') subrelations + */ inline static FF process_subrelation_evaluations(const RelationEvaluations& evals, const std::array& challenges, FF& linearly_dependent_contribution) diff --git a/barretenberg/cpp/src/barretenberg/relations/utils.hpp b/barretenberg/cpp/src/barretenberg/relations/utils.hpp index b4d6e35a3df..846fc1d5f9c 100644 --- a/barretenberg/cpp/src/barretenberg/relations/utils.hpp +++ b/barretenberg/cpp/src/barretenberg/relations/utils.hpp @@ -252,48 +252,6 @@ template class RelationUtils { apply_to_tuple_of_arrays(scale_by_challenges_and_accumulate, tuple); } - /** - * @brief Scales elements, representing evaluations of polynomials in subrelations, by separate challenges and then - * sum them together. This function has identical functionality with the one above with the caveat that one such - * evaluation is part of a linearly dependent subrelation and hence needs to be accumulated separately. - * - * @details Such functionality is needed when computing the evaluation of the full relation at a specific row in - * the execution trace because a linearly dependent subrelation does not act on a specific row but rather on the - * entire execution trace. - * - * @param tuple - * @param challenges - * @param current_scalar - * @param result - * @param linearly_dependent_contribution - */ - static void scale_and_batch_elements(auto& tuple, - const RelationSeparator& challenges, - FF current_scalar, - FF& result, - FF& linearly_dependent_contribution) - requires bb::IsFoldingFlavor - { - size_t idx = 0; - std::array tmp{ current_scalar }; - - std::copy(challenges.begin(), challenges.end(), tmp.begin() + 1); - - auto scale_by_challenge_and_accumulate = - [&](Element& element) { - using Relation = typename std::tuple_element_t; - const bool is_subrelation_linearly_independent = - bb::subrelation_is_linearly_independent(); - if (is_subrelation_linearly_independent) { - result += element * tmp[idx]; - } else { - linearly_dependent_contribution += element * tmp[idx]; - } - idx++; - }; - apply_to_tuple_of_arrays_elements(scale_by_challenge_and_accumulate, tuple); - } - /** * @brief Scale elements by consecutive powers of a given challenge then sum the result * @param result Batched result From f2d7c1fdc6137f8f9163d0123533a27d6bf785ee Mon Sep 17 00:00:00 2001 From: codygunton Date: Mon, 30 Sep 2024 00:36:12 +0000 Subject: [PATCH 6/6] Mark todo --- .../barretenberg/protogalaxy/protogalaxy_prover_internal.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp index 1bdb3ff6201..945c98ca088 100644 --- a/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp +++ b/barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_prover_internal.hpp @@ -72,6 +72,8 @@ template class ProtogalaxyProverInternal { const std::array& challenges, FF& linearly_dependent_contribution) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1115): Iniitalize with first subrelation value to + // avoid Montgomery allocating 0 and doing a mul. This is about 60ns per row. FF linearly_independent_contribution{ 0 }; size_t idx = 0;