-
Notifications
You must be signed in to change notification settings - Fork 284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Sequential insertion in indexed trees #10111
Merged
Merged
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
0801f37
add sequential insertion
sirasistant 18c052e
fixes
sirasistant 66ba930
refactor
sirasistant a3f2c0c
fix capture
sirasistant ac6498f
fix
sirasistant bc5ec50
Merge branch 'master' into arv/insert_sequential
sirasistant e9d4d43
fmt
sirasistant c10abc3
fix some units
sirasistant a526cbe
Merge branch 'arv/insert_sequential' of github.com:AztecProtocol/azte…
sirasistant 35b22b5
refactor rename
sirasistant debdc60
fix
sirasistant 014fb45
fix
sirasistant 906141b
fix
sirasistant a7ff1c3
Merge branch 'master' into arv/insert_sequential
sirasistant 2ea6fdf
document caching issue
sirasistant d0040d4
test and bench
sirasistant 507d14d
reorder bench
sirasistant 649d52d
Merge branch 'master' into arv/insert_sequential
sirasistant 34f10ef
docs
sirasistant 25c5bc5
remove batches in sync
sirasistant fca466e
fix type
sirasistant d50f8f1
Merge branch 'master' into arv/insert_sequential
sirasistant 716755f
add specific test for multiple updates to the same leaf in the same call
sirasistant df603fe
Merge branch 'master' into arv/insert_sequential
sirasistant 1b6462b
fix: empty leaf hashing
sirasistant d7191f8
Merge branch 'master' into arv/insert_sequential
sirasistant df773ad
Merge branch 'master' into arv/insert_sequential
sirasistant 413227f
fix corner case
sirasistant e4fa6a5
Merge branch 'arv/insert_sequential' of github.com:AztecProtocol/azte…
sirasistant b1b8552
Merge branch 'master' into arv/insert_sequential
sirasistant 66f64b1
Merge branch 'master' into arv/insert_sequential
sirasistant c12ab24
fix
sirasistant ce6297d
restore publicDataWritesCount
sirasistant fa2291e
Merge branch 'arv/insert_sequential' of github.com:AztecProtocol/azte…
sirasistant f4586ca
Merge branch 'master' into arv/insert_sequential
sirasistant 9d43a28
Merge branch 'master' into arv/insert_sequential
sirasistant 5adde1e
Merge branch 'master' into arv/insert_sequential
sirasistant 98ae886
Merge branch 'master' into arv/insert_sequential
sirasistant 1b74135
Merge branch 'master' into arv/insert_sequential
sirasistant d544e4c
use explicit fr::zero
sirasistant 92823b4
Merge branch 'master' into arv/insert_sequential
sirasistant d6324d6
Merge branch 'master' into arv/insert_sequential
sirasistant File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,24 +27,79 @@ const size_t MAX_BATCH_SIZE = 64; | |
template <typename TreeType> void add_values(TreeType& tree, const std::vector<NullifierLeafValue>& values) | ||
{ | ||
Signal signal(1); | ||
typename TreeType::AddCompletionCallback completion = [&](const auto&) -> void { signal.signal_level(0); }; | ||
bool success = true; | ||
std::string error_message; | ||
typename TreeType::AddCompletionCallback completion = [&](const auto& result) -> void { | ||
success = result.success; | ||
error_message = result.message; | ||
signal.signal_level(0); | ||
}; | ||
|
||
tree.add_or_update_values(values, completion); | ||
signal.wait_for_level(0); | ||
if (!success) { | ||
throw std::runtime_error(format("Failed to add values: ", error_message)); | ||
} | ||
} | ||
|
||
template <typename TreeType> void add_values_with_witness(TreeType& tree, const std::vector<NullifierLeafValue>& values) | ||
{ | ||
bool success = true; | ||
std::string error_message; | ||
Signal signal(1); | ||
typename TreeType::AddCompletionCallbackWithWitness completion = [&](const auto&) -> void { | ||
typename TreeType::AddCompletionCallbackWithWitness completion = [&](const auto& result) -> void { | ||
success = result.success; | ||
error_message = result.message; | ||
signal.signal_level(0); | ||
}; | ||
|
||
tree.add_or_update_values(values, completion); | ||
signal.wait_for_level(0); | ||
if (!success) { | ||
throw std::runtime_error(format("Failed to add values with witness: ", error_message)); | ||
} | ||
} | ||
|
||
template <typename TreeType> void add_values_sequentially(TreeType& tree, const std::vector<NullifierLeafValue>& values) | ||
{ | ||
bool success = true; | ||
std::string error_message; | ||
Signal signal(1); | ||
typename TreeType::AddCompletionCallback completion = [&](const auto& result) -> void { | ||
success = result.success; | ||
error_message = result.message; | ||
signal.signal_level(0); | ||
}; | ||
|
||
tree.add_or_update_values_sequentially(values, completion); | ||
signal.wait_for_level(0); | ||
if (!success) { | ||
throw std::runtime_error(format("Failed to add values sequentially: ", error_message)); | ||
} | ||
} | ||
|
||
template <typename TreeType> | ||
void add_values_sequentially_with_witness(TreeType& tree, const std::vector<NullifierLeafValue>& values) | ||
{ | ||
bool success = true; | ||
std::string error_message; | ||
Signal signal(1); | ||
typename TreeType::AddSequentiallyCompletionCallbackWithWitness completion = [&](const auto& result) -> void { | ||
success = result.success; | ||
error_message = result.message; | ||
signal.signal_level(0); | ||
}; | ||
|
||
tree.add_or_update_values_sequentially(values, completion); | ||
signal.wait_for_level(0); | ||
if (!success) { | ||
throw std::runtime_error(format("Failed to add values sequentially with witness: ", error_message)); | ||
} | ||
} | ||
|
||
template <typename TreeType> void multi_thread_indexed_tree_bench(State& state) noexcept | ||
enum InsertionStrategy { SEQUENTIAL, BATCH }; | ||
|
||
template <typename TreeType, InsertionStrategy strategy> void multi_thread_indexed_tree_bench(State& state) noexcept | ||
{ | ||
const size_t batch_size = size_t(state.range(0)); | ||
const size_t depth = TREE_DEPTH; | ||
|
@@ -61,10 +116,14 @@ template <typename TreeType> void multi_thread_indexed_tree_bench(State& state) | |
|
||
const size_t initial_size = 1024 * 16; | ||
std::vector<NullifierLeafValue> initial_batch(initial_size); | ||
for (size_t i = 0; i < batch_size; ++i) { | ||
for (size_t i = 0; i < initial_size; ++i) { | ||
initial_batch[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
add_values(tree, initial_batch); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, initial_batch); | ||
} else { | ||
add_values(tree, initial_batch); | ||
} | ||
|
||
for (auto _ : state) { | ||
state.PauseTiming(); | ||
|
@@ -73,11 +132,15 @@ template <typename TreeType> void multi_thread_indexed_tree_bench(State& state) | |
values[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
state.ResumeTiming(); | ||
add_values(tree, values); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, values); | ||
} else { | ||
add_values(tree, values); | ||
} | ||
} | ||
} | ||
|
||
template <typename TreeType> void single_thread_indexed_tree_bench(State& state) noexcept | ||
template <typename TreeType, InsertionStrategy strategy> void single_thread_indexed_tree_bench(State& state) noexcept | ||
{ | ||
const size_t batch_size = size_t(state.range(0)); | ||
const size_t depth = TREE_DEPTH; | ||
|
@@ -94,10 +157,14 @@ template <typename TreeType> void single_thread_indexed_tree_bench(State& state) | |
|
||
const size_t initial_size = 1024 * 16; | ||
std::vector<NullifierLeafValue> initial_batch(initial_size); | ||
for (size_t i = 0; i < batch_size; ++i) { | ||
for (size_t i = 0; i < initial_size; ++i) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That look like a bug, nice catch |
||
initial_batch[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
add_values(tree, initial_batch); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, initial_batch); | ||
} else { | ||
add_values(tree, initial_batch); | ||
} | ||
|
||
for (auto _ : state) { | ||
state.PauseTiming(); | ||
|
@@ -106,11 +173,16 @@ template <typename TreeType> void single_thread_indexed_tree_bench(State& state) | |
values[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
state.ResumeTiming(); | ||
add_values(tree, values); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, values); | ||
} else { | ||
add_values(tree, values); | ||
} | ||
} | ||
} | ||
|
||
template <typename TreeType> void multi_thread_indexed_tree_with_witness_bench(State& state) noexcept | ||
template <typename TreeType, InsertionStrategy strategy> | ||
void multi_thread_indexed_tree_with_witness_bench(State& state) noexcept | ||
{ | ||
const size_t batch_size = size_t(state.range(0)); | ||
const size_t depth = TREE_DEPTH; | ||
|
@@ -127,10 +199,14 @@ template <typename TreeType> void multi_thread_indexed_tree_with_witness_bench(S | |
|
||
const size_t initial_size = 1024 * 16; | ||
std::vector<NullifierLeafValue> initial_batch(initial_size); | ||
for (size_t i = 0; i < batch_size; ++i) { | ||
for (size_t i = 0; i < initial_size; ++i) { | ||
initial_batch[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
add_values(tree, initial_batch); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, initial_batch); | ||
} else { | ||
add_values(tree, initial_batch); | ||
} | ||
|
||
for (auto _ : state) { | ||
state.PauseTiming(); | ||
|
@@ -139,11 +215,16 @@ template <typename TreeType> void multi_thread_indexed_tree_with_witness_bench(S | |
values[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
state.ResumeTiming(); | ||
add_values_with_witness(tree, values); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially_with_witness(tree, values); | ||
} else { | ||
add_values_with_witness(tree, values); | ||
} | ||
} | ||
} | ||
|
||
template <typename TreeType> void single_thread_indexed_tree_with_witness_bench(State& state) noexcept | ||
template <typename TreeType, InsertionStrategy strategy> | ||
void single_thread_indexed_tree_with_witness_bench(State& state) noexcept | ||
{ | ||
const size_t batch_size = size_t(state.range(0)); | ||
const size_t depth = TREE_DEPTH; | ||
|
@@ -160,10 +241,14 @@ template <typename TreeType> void single_thread_indexed_tree_with_witness_bench( | |
|
||
const size_t initial_size = 1024 * 16; | ||
std::vector<NullifierLeafValue> initial_batch(initial_size); | ||
for (size_t i = 0; i < batch_size; ++i) { | ||
for (size_t i = 0; i < initial_size; ++i) { | ||
initial_batch[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
add_values(tree, initial_batch); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially(tree, initial_batch); | ||
} else { | ||
add_values(tree, initial_batch); | ||
} | ||
|
||
for (auto _ : state) { | ||
state.PauseTiming(); | ||
|
@@ -172,53 +257,105 @@ template <typename TreeType> void single_thread_indexed_tree_with_witness_bench( | |
values[i] = fr(random_engine.get_random_uint256()); | ||
} | ||
state.ResumeTiming(); | ||
add_values_with_witness(tree, values); | ||
if (strategy == SEQUENTIAL) { | ||
add_values_sequentially_with_witness(tree, values); | ||
} else { | ||
add_values_with_witness(tree, values); | ||
} | ||
} | ||
} | ||
|
||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2>) | ||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2>) | ||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2>) | ||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2>) | ||
BENCHMARK(single_thread_indexed_tree_with_witness_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2>) | ||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2>) | ||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2>) | ||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_with_witness_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(single_thread_indexed_tree_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(10); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2, BATCH>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
->Iterations(100); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(2, MAX_BATCH_SIZE) | ||
->Iterations(1000); | ||
|
||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2>) | ||
BENCHMARK(multi_thread_indexed_tree_bench<Poseidon2, SEQUENTIAL>) | ||
->Unit(benchmark::kMillisecond) | ||
->RangeMultiplier(2) | ||
->Range(512, 8192) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the sequential insertion compare with the batch insertion?
The mutli-threaded batch insertion doesn't perform as well as I had hoped. It was much better when we used pedersen as a much greater portion of the time was spent hashing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With witnesses, it's worse (that's expected since you're fetching more witnesses)
However, without witnesses, it's better single threaded (yay) but worse multithreaded. That one is a weird one, since I expected sparse_batch_update (which is the only thing sequential insertion without witnesses does) to perform better than sparse_batch_update and then append. Do you know why this could be the case? Should I profile this thing?
Here is the bench results I got on mainframe (not the best place for benching but good enough I hope)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed. Looks like the multi-threaded performs a little better in most scenarios. I think we should wait until the compete use case of the world state has settled further and then optimise.