Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add threading to add block #1637

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

add threading to add block #1637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/wallet3/transaction_scanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace wallet

// A derivation is simply the private view key multiplied by the tx public key
// do this for every tx public key in the transaction
//TODO SEAN THIS IS A BOTTLENECK
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking here: what if we used a threadpool kind of entity where we put heavy but pure jobs onto. then have the these heavy functions (if they can be pure) return a std::future that will be fulfilled when the operation is done in the thread pool. we could have those fan out onto as many threads as we want.

the following idea could look something looking like:

auto ftr_derivations = wallet_keys->async_generate_key_derivation(tx_public_keys);
auto derivation = ftr_derivations.get();

auto derivations = wallet_keys->generate_key_derivations(tx_public_keys);
bool coinbase_transaction = cryptonote::is_coinbase(tx.tx);
// Output belongs to public key derived as follows:
Expand All @@ -55,6 +56,7 @@ namespace wallet
std::optional<cryptonote::subaddress_index> sub_index{std::nullopt};
for (derivation_index = 0; derivation_index < derivations.size(); derivation_index++)
{
//TODO SEAN THIS IS A BOTTLENECK
sub_index = wallet_keys->output_and_derivation_ours(
derivations[derivation_index], output_target->key, output_index);
if (sub_index)
Expand Down
18 changes: 14 additions & 4 deletions src/wallet3/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ namespace wallet
Wallet::add_block(const Block& block)
{
oxen::log::trace(logcat, "add block called with block height {}", block.height);
auto db_tx = db->db_transaction();

db->store_block(block);

Expand All @@ -164,9 +163,7 @@ namespace wallet
}
}

db_tx.commit();

last_scan_height++;
}

void
Expand All @@ -185,11 +182,24 @@ namespace wallet
return;
}

std::vector<std::thread> threads;

auto db_tx = db->db_transaction();
for (const auto& block : blocks)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we could std::move the block in this foreach into that lambda, we can capture [this, block=std::move(block)] we can avoid a capture all by reference (which is something i always try to avoid using). that or capture with [this, &block]

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the insides of add_block sound possibly thread unsafe.

do we mark all functions that are pure with const or no?

{
if (block.height == last_scan_height + 1)
add_block(block);
{
threads.emplace_back([&]() {
add_block(block);
});
last_scan_height++;
}
}
// Wait for all threads to finish
for (auto& t : threads) {
t.join();
}
db_tx.commit();
daemon_comms->register_wallet(*this, last_scan_height + 1 /*next needed block*/, false);
}

Expand Down