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

Use shared ProcessPoolExecutor across all services #1157

Merged

Conversation

pipermerriam
Copy link
Member

@pipermerriam pipermerriam commented Aug 8, 2018

revamp of #1155

What was wrong?

fixes #1076

In the chain and state syncers there is an API for running computationally heavy workloads in a process pool of workers. The worker pool is automatically sized according to the number of cpus available on the machine. This pattern is quite useful, however, if used concurrently by two services, it would result in multiple pools being created, and would likely overwhelm the CPU load on the machine.

How was it fixed?

The _run_in_executor API is now available for all BaseService subclasses. This implements a quick-and-easy solution using a global executor pool which will be re-used across all BaseService subclasses within a single process.

This PR also contains additional cleanup:

  • The BasePeer class is now instantiated with the cancel token from the PeerPool.
  • More consistent usage of the BaseService.loop in the various asyncio APIs.
  • Minor linting cleanup

Cute Animal Picture

unlikely-sleeping-buddies-animal-friendship-301__880

@pipermerriam pipermerriam changed the title Piper/service context api Use shared ProcessPoolExecutor across all services Aug 8, 2018
Copy link
Contributor

@carver carver left a comment

Choose a reason for hiding this comment

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

This approach feels much better. 👍

@settings(max_examples=10)
@settings(
max_examples=10,
deadline=4000,
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, was this taking a really long time with just a few examples?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's because it runs 10 examples, each which populate a trie with 1000 nodes, which as we know is a computationally expensive operation.

@pipermerriam pipermerriam merged commit 01d327f into ethereum:master Aug 8, 2018
@pipermerriam pipermerriam deleted the piper/service-context-api branch August 8, 2018 17:33
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.

Formalize API and use common executor pool for p2p.Service.run_in_executor
2 participants