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.

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

Move prewarming as early as possible #8001

Merged
merged 8 commits into from
Jan 6, 2025
Merged

Move prewarming as early as possible #8001

merged 8 commits into from
Jan 6, 2025

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Jan 3, 2025

Changes

  • Move prewarming as early as possible in block processing (every little helps)

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • No

@benaadams benaadams requested a review from LukaszRozmej January 3, 2025 16:06
@LukaszRozmej
Copy link
Member

Is it worth it though? How much do we gain on normal flow?

@benaadams
Copy link
Member Author

benaadams commented Jan 4, 2025

Is it worth it though? How much do we gain on normal flow?

Means it gets the prewarmer threads queued to ThreadPool before TxHashCalculator and all the callback events.

Every microsecond counts for the early prewarms (beacon root and first txs)

@LukaszRozmej
Copy link
Member

ok does it register on our grafana graphs?

@benaadams
Copy link
Member Author

ok does it register on our grafana graphs?

Yes though can't find it atm (am doing a full sync)

We want the prewarming to front run the txs.

Block takes 10ms running 300 tx, so roughly 33 microseconds per tx. At that speed any and all edge is significant; as to whether the tx is prewarmed or not.

This I think is the maximum it can go ahead (I moved it earlier but was unhappy that some of the block wasn't set in state, so missing trienode errors)

@benaadams
Copy link
Member Author

Moved TxHashCalculator.CalculateInBackground to last item as that's not needed until forkchoice, so less over subscription on cpu threads

Comment on lines 120 to 126
skipPrewarming = preWarmer is null || suggestedBlock.Transactions.Length < 3;
// If cancelationTokenSource is not null it means we are in first iteration of loop
// and started prewarming at method entry, so don't start it again
if (cancellationTokenSource is null && !skipPrewarming)
{
(cancellationTokenSource, preWarmTask) = PreWarmTransactions(suggestedBlock, preBlockStateRoot);
}
Copy link
Member

Choose a reason for hiding this comment

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

This code is very similar to the one above, can we have a helper method?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved into PreWarm method

@benaadams benaadams merged commit b9ed309 into master Jan 6, 2025
79 checks passed
@benaadams benaadams deleted the prewarm-early branch January 6, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants