From 8ab7344a32aa4ff426220be95380dd6fa2f228a0 Mon Sep 17 00:00:00 2001 From: Martun Karapetyan Date: Mon, 5 Aug 2024 15:08:05 +0400 Subject: [PATCH] Remove some unnecessary TODOs. --- .../algebra/fields/detail/element/fp.hpp | 8 -- .../detail/element/fp12_2over3over2.hpp | 2 +- .../algebra/fields/detail/element/fp2.hpp | 14 +- libs/algebra/test/bench_test/bench_fields.cpp | 129 +++++++++--------- .../math/polynomial/polynomial_dfs.hpp | 1 - .../modular/modular_adaptor.hpp | 1 - .../test/modular_adaptor_fixed.cpp | 1 - 7 files changed, 69 insertions(+), 87 deletions(-) diff --git a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp.hpp b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp.hpp index 110f91da2f..86cfa50afc 100644 --- a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp.hpp +++ b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp.hpp @@ -121,16 +121,12 @@ namespace nil { } constexpr element_fp &operator-=(const element_fp &B) { - // TODO(martun): consider directly taking the backend and calling - // eval_add to improve performance. data -= B.data; return *this; } constexpr element_fp &operator+=(const element_fp &B) { - // TODO(martun): consider directly taking the backend and calling - // eval_add to improve performance. data += B.data; return *this; @@ -224,10 +220,6 @@ namespace nil { return element_fp(inverse_mod(data)); } - // TODO: complete method - constexpr element_fp _2z_add_3x() { - } - constexpr element_fp squared() const { return element_fp(data * data); // maybe can be done more effective } diff --git a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp12_2over3over2.hpp b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp12_2over3over2.hpp index 3fac0d9604..0880580bde 100644 --- a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp12_2over3over2.hpp +++ b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp12_2over3over2.hpp @@ -145,7 +145,7 @@ namespace nil { element_fp12_2over3over2 sqrt() const { - // compute squared root with Tonelli--Shanks + // TODO: compute squared root with Tonelli--Shanks } element_fp12_2over3over2 squared() const { diff --git a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp2.hpp b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp2.hpp index dded2e43f0..5d8d1bd984 100644 --- a/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp2.hpp +++ b/libs/algebra/include/nil/crypto3/algebra/fields/detail/element/fp2.hpp @@ -133,8 +133,6 @@ namespace nil { } constexpr element_fp2 operator*(const element_fp2 &B) const { - // TODO: the use of data and B.data directly in return statement addition cause constexpr - // error for gcc const underlying_type A0 = data[0], A1 = data[1], B0 = B.data[0], B1 = B.data[1]; const underlying_type A0B0 = data[0] * B.data[0], A1B1 = data[1] * B.data[1]; @@ -185,10 +183,6 @@ namespace nil { return element_fp2(-(data[1] * y_b), data[0] * y_b); } - constexpr element_fp2 _2z_add_3x() { - return element_fp2(data[0]._2z_add_3x(), data[1]._2z_add_3x()); - } - constexpr element_fp2 divBy2() const { return element_fp2(divBy2(data[0]), divBy2(data[1])); } @@ -245,12 +239,9 @@ namespace nil { } constexpr element_fp2 squared() const { - // return (*this) * (*this); // maybe can be done more effective - /* Devegili OhEig Scott Dahab --- Multiplication and Squaring on Pairing-Friendly * Fields.pdf; Section 3 (Complex squaring) */ - // TODO: reference here could cause error in constexpr for gcc - const underlying_type A = data[0], B = data[1]; + const underlying_type& A = data[0], B = data[1]; const underlying_type AB = A * B; return element_fp2((A + B) * (A + non_residue * B) - AB - non_residue * AB, AB + AB); @@ -261,8 +252,7 @@ namespace nil { /* Devegili OhEig Scott Dahab --- Multiplication and Squaring on Pairing-Friendly * Fields.pdf; Section 3 (Complex squaring) */ - // TODO: reference here could cause error in constexpr for gcc - const underlying_type A = data[0], B = data[1]; + const underlying_type& A = data[0], B = data[1]; const underlying_type AB = A * B; data[0] = (A + B) * (A + non_residue * B) - AB - non_residue * AB; diff --git a/libs/algebra/test/bench_test/bench_fields.cpp b/libs/algebra/test/bench_test/bench_fields.cpp index 84750d1e7a..e63a761ccc 100644 --- a/libs/algebra/test/bench_test/bench_fields.cpp +++ b/libs/algebra/test/bench_test/bench_fields.cpp @@ -58,6 +58,42 @@ using namespace nil::crypto3::algebra; BOOST_AUTO_TEST_SUITE(fields_manual_tests) +template +std::vector> gather_stats( + std::vector& points1, + std::vector& points2, + std::function& result, + const std::vector& samples, + std::size_t sample)> operation, + size_t samples_per_batch, + const std::string& operation_name, + size_t BATCHES = 1000) { + using duration = std::chrono::duration; + + std::vector batch_duration(BATCHES); + auto save = points1[3]; + + for(size_t b = 0; b < BATCHES; ++b) { + auto start = std::chrono::high_resolution_clock::now(); + for(size_t i = 0; i < samples_per_batch; ++i) { + operation(points1, points2, i); + } + + auto finish = std::chrono::high_resolution_clock::now(); + batch_duration[b] = (finish - start) * 1.0 / samples_per_batch; + } + + // prevent value 'result' from optimizating out + std::cerr << save << std::endl; + + auto average_duration = std::accumulate(batch_duration.begin(), batch_duration.end(), duration(0)) / batch_duration.size(); + std::cout << "Average time for operator " << operation_name << ": " + << std::fixed << std::setprecision(3) << average_duration.count() / samples_per_batch + << " ns" << std::endl; + + return batch_duration; +}; + template void run_perf_test(std::string const& field_name) { using namespace nil::crypto3; @@ -71,93 +107,61 @@ void run_perf_test(std::string const& field_name) { // size of arrays is 4 times larger than typical L1 data cache size_t SAMPLES_COUNT = 4*32*1024/sizeof(value_type); - for (int i = 0; i < SAMPLES_COUNT; ++i) { + for (std::size_t i = 0; i < SAMPLES_COUNT; ++i) { points1.push_back(algebra::random_element()); points2.push_back(algebra::random_element()); } - auto gather_stats = [&points1, &points2] - (std::function & result, std::vector const& samples, std::size_t sample)> operation, - size_t samples_per_batch, const std::string& operation_name) { - size_t BATCHES = 1000; - - using duration = std::chrono::duration; - - std::vector batch_duration; - batch_duration.resize(BATCHES); - auto save = points1[3];; - - for(size_t b = 0; b < BATCHES; ++b) { - // if (b % (BATCHES/10) == 0) std::cerr << "Batch progress:" << b << std::endl; - auto start = std::chrono::high_resolution_clock::now(); - auto points_index = 0; - - for(size_t i = 0; i < samples_per_batch; ++i) { - operation(points1, points2, i); - ++points_index; - if (points_index == 1000) - points_index = 0; - } + const size_t BATCHES = 1000; - auto finish = std::chrono::high_resolution_clock::now(); - batch_duration[b] = (finish - start) * 1.0 / samples_per_batch; - } - - // prevent value 'result' from optimizating out - std::cerr << save << std::endl; - - auto s = batch_duration[0]; - for(size_t b = 1; b < batch_duration.size(); ++b) { - s += batch_duration[b]; - } - - s /= batch_duration.size() - 2; - std::cout << "Average time for operator " << operation_name << ": " << std::fixed << std::setprecision(3) << s.count() << std::endl; - - return batch_duration; - }; - - - for(int mult = 1; mult <= 100; ++mult) { - int MULTIPLICATOR = mult; - std::cout << "MULT: " << MULTIPLICATOR << std::endl; - - auto plus_results = gather_stats( + int mult = 100; + auto plus_results = gather_stats( + points1, + points2, [&](std::vector &result, std::vector const& samples, std::size_t sample) { - for(int m = 0; m < MULTIPLICATOR; m++) + for(int m = 0; m < mult; m++) result[sample*(sample+m) % SAMPLES_COUNT] += samples[sample*(sample+m)*17 % SAMPLES_COUNT]; - }, 10000 / MULTIPLICATOR, "Addition"); + }, 10000 / mult, "Addition", BATCHES); - auto mul_results = gather_stats( + auto mul_results = gather_stats( + points1, + points2, [&](std::vector &result, std::vector const& samples, std::size_t sample) { - for(int m = 0; m < MULTIPLICATOR; m++) + for(int m = 0; m < mult; m++) result[sample*(sample+m) % SAMPLES_COUNT] *= samples[sample*(sample+m)*17 % SAMPLES_COUNT]; - }, 1000 / MULTIPLICATOR, "Multiplication"); + }, 1000 / mult, "Multiplication", BATCHES); - auto minus_results = gather_stats( + auto minus_results = gather_stats( + points1, + points2, [&](std::vector &result, std::vector const& samples, std::size_t sample) { - for(int m = 0; m < MULTIPLICATOR; m++) + for(int m = 0; m < mult; m++) result[sample*(sample+m) % SAMPLES_COUNT] -= samples[sample*(sample+m)*17 % SAMPLES_COUNT]; - }, 10000 / MULTIPLICATOR, "Subtraction"); + }, 10000 / mult, "Subtraction", BATCHES); - auto sqr_results = gather_stats( + auto sqr_results = gather_stats( + points1, + points2, [&](std::vector &result, std::vector const& samples, std::size_t sample) { - for(int m = 0; m < MULTIPLICATOR; m++) + for(int m = 0; m < mult; m++) result[sample*(sample+m) % SAMPLES_COUNT].square_inplace(); - }, 1000 / MULTIPLICATOR, "Square In-Place"); + }, 1000 / mult, "Square In-Place", BATCHES); - auto inv_results = gather_stats( + auto inv_results = gather_stats( + points1, + points2, [&](std::vector &result, std::vector const& samples, std::size_t sample) { - for(int m = 0; m < MULTIPLICATOR; m++) + for(int m = 0; m < mult; m++) result[sample*(sample+m) % SAMPLES_COUNT] = samples[sample*(sample+m)*17 % SAMPLES_COUNT].inversed(); - }, 100 / MULTIPLICATOR, "Inverse"); + }, 100 / mult, "Inverse", BATCHES); + char filename[200]= {0}; - sprintf(filename,"%s-stats-%03d.csv", field_name.c_str(), MULTIPLICATOR); + sprintf(filename,"%s-stats-%03d.csv", field_name.c_str(), mult); std::ofstream f(filename, std::ofstream::out); f << "# " << typeid(Field).name() << std::endl; @@ -173,7 +177,6 @@ void run_perf_test(std::string const& field_name) { } f.close(); - } } BOOST_AUTO_TEST_CASE(field_operation_perf_test_pallas) { diff --git a/libs/math/include/nil/crypto3/math/polynomial/polynomial_dfs.hpp b/libs/math/include/nil/crypto3/math/polynomial/polynomial_dfs.hpp index a027209763..2b5a9c6e47 100644 --- a/libs/math/include/nil/crypto3/math/polynomial/polynomial_dfs.hpp +++ b/libs/math/include/nil/crypto3/math/polynomial/polynomial_dfs.hpp @@ -121,7 +121,6 @@ namespace nil { BOOST_ASSERT_MSG(val.size() == detail::power_of_two(val.size()), "DFS optimal polynomial size must be a power of two"); } - // TODO: add constructor with omega polynomial_dfs(polynomial_dfs&& x) BOOST_NOEXCEPT(std::is_nothrow_move_constructible::value) diff --git a/libs/multiprecision/include/nil/crypto3/multiprecision/modular/modular_adaptor.hpp b/libs/multiprecision/include/nil/crypto3/multiprecision/modular/modular_adaptor.hpp index 8415535ec3..51aa7ad24c 100644 --- a/libs/multiprecision/include/nil/crypto3/multiprecision/modular/modular_adaptor.hpp +++ b/libs/multiprecision/include/nil/crypto3/multiprecision/modular/modular_adaptor.hpp @@ -182,7 +182,6 @@ namespace boost { return eval_is_zero(val.base_data()); } - // TODO: check returned value template BOOST_MP_CXX14_CONSTEXPR int eval_get_sign(const modular_adaptor &) { return 1; diff --git a/libs/multiprecision/test/modular_adaptor_fixed.cpp b/libs/multiprecision/test/modular_adaptor_fixed.cpp index 598e3f1a66..a4870af401 100644 --- a/libs/multiprecision/test/modular_adaptor_fixed.cpp +++ b/libs/multiprecision/test/modular_adaptor_fixed.cpp @@ -80,7 +80,6 @@ constexpr void pow_test(const boost::multiprecision::number bool base_operations_test(std::array test_set) {