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

Add option to have multiple network processing thread #5749

Merged
merged 5 commits into from
Jun 1, 2023

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Jun 1, 2023

  • Currently the network thread is essentially single threaded with some request spawning other task.
  • This add an option to allow setting more than 1 thread.
  • On faster internet (> 500Mbps), this becaomes a bottleneck on old blocks.
  • However, the threads kinda acts as a throttling mechanism which prevent external request from affecting block processing.

Changes

  • Add option to set more than 1 network thread.
  • Add tests to make sure frame encoder/decoder can run in parallel of each other.

Types of changes

What types of changes does your code introduce?

  • New feature (a non-breaking change that adds functionality)
  • Optimization

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Optional. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

Would Networking be faster if for blocks and receipts we wouldn't do a full deserialization and allocating memory, but instead use something like ReceiptsIterator (and implement BlockIterator) for that and then dump the bytes directly to DB? This could save CPU cycles, allocations, GC ect.

@@ -73,7 +74,7 @@ TimeSpan sendLatency
// new LoggerFilterOptions { MinLevel = Microsoft.Extensions.Logging.LogLevel.Warning });
// InternalLoggerFactory.DefaultFactory = loggerFactory;

_group = new SingleThreadEventLoop();
_group = new MultithreadEventLoopGroup(networkProcessingThread);
Copy link
Member

@LukaszRozmej LukaszRozmej Jun 1, 2023

Choose a reason for hiding this comment

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

should there be a check that if it is 1 it would spawn SingleThreadEventLoop ? Maybe it is more optimized than MultithreadEventLoopGroup(1)?

@asdacap
Copy link
Contributor Author

asdacap commented Jun 1, 2023

For blocks, at least during sync, the main bottleneck is likely IO. I guess, going straight to db would reduce some CPU, but I suspect its not going to change much. For receipts, the main bottleneck is the receipt verification. I tried a custom task scheduler before and it can saturate the whole CPU suggesting some scheduling inefficiency. I doubt other part is significant.

@asdacap asdacap merged commit 3378d8a into master Jun 1, 2023
@asdacap asdacap deleted the perf/turn-on-multithread-on-network branch June 1, 2023 10:58
@LukaszRozmej
Copy link
Member

For blocks, at least during sync, the main bottleneck is likely IO. I guess, going straight to db would reduce some CPU, but I suspect its not going to change much. For receipts, the main bottleneck is the receipt verification. I tried a custom task scheduler before and it can saturate the whole CPU suggesting some scheduling inefficiency. I doubt other part is significant.

as receipt thing is available I will try doing it some time.

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.

3 participants