Skip to content

Make txpool test run faster #9382

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

Merged
asdacap merged 7 commits into from
Oct 2, 2025
Merged

Make txpool test run faster #9382

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0096352
Make txpool test run faster
asdacap Sep 30, 2025
48c3102
Fix randomly failing pyspec test
asdacap Sep 30, 2025
f84c5ea
parallelize BroadcasterTests more
LukaszRozmej Oct 2, 2025
99f543a
Even more
LukaszRozmej Oct 2, 2025
8ec37c8
Parallelize more
LukaszRozmej Oct 2, 2025
77e0280
Remove this part
asdacap Oct 2, 2025
78ab65d
more parallel
LukaszRozmej Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Nethermind/Nethermind.State/StateProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,9 @@ public void Commit(IReleaseSpec releaseSpec, IWorldStateTracer stateTracer, bool
{
FlushToTree();
}

codeFlushTask.GetAwaiter().GetResult();

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

Using GetAwaiter().GetResult() can cause thread pool starvation and deadlocks. Consider using await if the method can be made async, or use ConfigureAwait(false) to avoid potential deadlocks: codeFlushTask.ConfigureAwait(false).GetAwaiter().GetResult().

Copilot uses AI. Check for mistakes.

@LukaszRozmej LukaszRozmej Sep 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is explicit optimization by @benaadams not to do it here, but in the background, then in Commit you have wait for the one from previous block.

@asdacap asdacap Sep 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Really? Then what is with the other wait at the end of the same function?

@benaadams benaadams Oct 1, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a PR that moves it to end of block processing; though is horribly merged conflicted atm #9012

@LukaszRozmej LukaszRozmej Oct 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So @benaadams is this PR acceptable or makes your solution unachievable and you are confident you will revisit it?

return;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.TxPool.Test/NonceManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void should_reuse_nonce_if_tx_rejected()
}

[Test]
[Repeat(10)]
[Repeat(2)]
public void should_lock_on_same_account()
{
using NonceLocker locker = _nonceManager.ReserveNonce(TestItem.AddressA, out UInt256 nonce);
Expand Down
10 changes: 8 additions & 2 deletions src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ public void should_skip_blob_txs_when_picking_best_persistent_txs_to_broadcast([
int addedTxsCount = TestItem.PrivateKeys.Length;
Transaction[] transactions = new Transaction[addedTxsCount];

for (int i = 0; i < addedTxsCount; i++)
Parallel.For(0, addedTxsCount, i =>
{
bool isBlob = i % 10 == 0;
transactions[i] = Build.A.Transaction
Expand All @@ -265,7 +265,10 @@ public void should_skip_blob_txs_when_picking_best_persistent_txs_to_broadcast([
.WithShardBlobTxTypeAndFieldsIfBlobTx()
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeys[i])
.TestObject;
});

for (int i = 0; i < addedTxsCount; i++)
{

@LukaszRozmej LukaszRozmej Sep 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Broadcast should be thread safe so you can put it under Parallel.For

@marcindsobczak marcindsobczak Sep 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At least it should be - order doesn't matter here so worth to move to parallelized part and verify it

_broadcaster.Broadcast(transactions[i], true);
}

Expand Down Expand Up @@ -423,14 +426,17 @@ public void should_not_pick_blob_txs_with_MaxFeePerBlobGas_lower_than_CurrentFee
int addedTxsCount = TestItem.PrivateKeys.Length;
Transaction[] transactions = new Transaction[addedTxsCount];

for (int i = 0; i < addedTxsCount; i++)
Parallel.For(0, addedTxsCount, i =>
{
transactions[i] = Build.A.Transaction
.WithShardBlobTxTypeAndFields()
.WithMaxFeePerBlobGas(i.GWei())
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeys[i])
Comment thread
benaadams marked this conversation as resolved.
.TestObject;
});

for (int i = 0; i < addedTxsCount; i++)
{
Comment on lines +435 to +436

@LukaszRozmej LukaszRozmej Sep 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

_broadcaster.Broadcast(transactions[i], true);
}

Expand Down
18 changes: 12 additions & 6 deletions src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.Blobs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,22 @@ public void should_reject_txs_with_nonce_too_far_in_future(TxType txType, int ma
};
_txPool = CreatePool(txPoolConfig, GetCancunSpecProvider());
EnsureSenderBalance(TestItem.AddressA, UInt256.MaxValue);
for (int nonce = 0; nonce < txPoolConfig.Size; nonce++)

Transaction[] txs = new Transaction[txPoolConfig.Size];
Parallel.For(0, txPoolConfig.Size, (nonce) =>
{
Transaction tx = Build.A.Transaction
txs[nonce] = Build.A.Transaction
.WithNonce((UInt256)nonce)
.WithType(txType)
.WithShardBlobTxTypeAndFieldsIfBlobTx()
.WithMaxFeePerGas(1.GWei())
.WithMaxPriorityFeePerGas(1.GWei())
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject;
Comment thread
benaadams marked this conversation as resolved.
});

_txPool.SubmitTx(tx, TxHandlingOptions.None).Should().Be(nonce > expectedNumberOfAcceptedTxs
for (int nonce = 0; nonce < txPoolConfig.Size; nonce++)
{
_txPool.SubmitTx(txs[nonce], TxHandlingOptions.None).Should().Be(nonce > expectedNumberOfAcceptedTxs
Comment thread
LukaszRozmej marked this conversation as resolved.
? AcceptTxResult.NonceTooFarInFuture
: AcceptTxResult.Accepted);
}
Expand Down Expand Up @@ -775,7 +780,8 @@ public void should_index_blobs_when_adding_txs([Values(true, false)] bool isPers
public void should_handle_indexing_blobs_when_adding_txs_in_parallel([Values(true, false)] bool isPersistentStorage)
{
const int txsPerSender = 10;
int poolSize = TestItem.PrivateKeys.Length * txsPerSender;
PrivateKey[] testPrivateKeys = TestItem.PrivateKeys[..64];
int poolSize = testPrivateKeys.Length * txsPerSender;
TxPoolConfig txPoolConfig = new()
{
BlobsSupport = isPersistentStorage ? BlobsSupportMode.Storage : BlobsSupportMode.InMemory,
Expand All @@ -791,13 +797,13 @@ public void should_handle_indexing_blobs_when_adding_txs_in_parallel([Values(tru

byte[] expectedBlobVersionedHash = null;

foreach (PrivateKey privateKey in TestItem.PrivateKeys)
foreach (PrivateKey privateKey in testPrivateKeys)
{
EnsureSenderBalance(privateKey.Address, UInt256.MaxValue);
}

// adding, getting and removing txs in parallel
Parallel.ForEach(TestItem.PrivateKeys, privateKey =>
Parallel.ForEach(testPrivateKeys, privateKey =>
{
for (int i = 0; i < txsPerSender; i++)
{
Expand Down