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

Make sure we can reach the user's requested FVM concurrency #449

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

Stebalien
Copy link
Member

We previously used the FVM's ThreadedExecutor to execute messages on separate threads because the FVM requires 64MiB of stack space.

  1. The FVM v3 supported for 8 concurrent threads.
  2. The FVM v4 supports up to the number of CPU threads available.

Unfortunately, neither version was influenced by the LOTUS_FVM_CONCURRENCY environment variable.

This patch fixes this by:

  1. Moving the thread-pool to the FFI itself (sharing it between FVM versions).
  2. Setting the thread-pool size equal to LOTUS_FVM_CONCURRENCY.

It also defaults LOTUS_FVM_CONCURRENCY to the number of available CPU threads instead of the previous 4.

NOTE: I've also tried increasing the stack size instead of using threads, but Go does not like it when other foreign mess with the stack size of its threads (but it has no problem if we create our own threads).

fixes filecoin-project/lotus#11817

replaces #448

We previously used the FVM's `ThreadedExecutor` to execute messages on
separate threads because the FVM requires 64MiB of stack space.

1. The FVM v3 supported for 8 concurrent threads.
2. The FVM v4 supports up to the number of CPU threads available.

Unfortunately, neither version was influenced by the
`LOTUS_FVM_CONCURRENCY` environment variable.

This patch fixes this by:

1. Moving the thread-pool to the FFI itself (sharing it between FVM
versions).
2. Setting the thread-pool size equal to `LOTUS_FVM_CONCURRENCY`.

It also defaults `LOTUS_FVM_CONCURRENCY` to the number of available
CPU threads instead of the previous 4.

NOTE: I've also tried increasing the stack size instead of using
threads, but Go _does not_ like it when other foreign mess with the
stack size of _its_ threads (but it has no problem if we create our own
threads).

fixes filecoin-project/lotus#11817
@Stebalien Stebalien force-pushed the steb/actual-concurrency branch from 255a0a9 to c6296ec Compare April 6, 2024 18:49
Stebalien added a commit to filecoin-project/lotus that referenced this pull request Apr 6, 2024
@Stebalien
Copy link
Member Author

@vbaranov (sorry, I'm not sure who I should be pinging).

When trying to give the FVM "saner defaults" with respect to concurrency to make indexing the Filecoin chain less painful, I came across another issue. For epochs 2683348 to 3469380 (FVMv3), the FVM will be limited to at most 8 parallel executions even if you raise LOTUS_FVM_CONCURRENCY past 8 (defaults to 4), as discussed in our call. FVMv4 (epoch 3469380 onwards) is limited to the number of CPU threads available to the process.

This patch:

  1. Sets the default concurrency to the number of CPU threads available. So, even if you don't set LOTUS_FVM_CONCURRENCY, you should get saner defaults.
  2. Ensures that setting LOTUS_FVM_CONCURRENCY will actually give you that much parallelism.

I've created a lotus branch if you want to test it out: https://github.com/filecoin-project/lotus/tree/steb/actual-concurrency

rust/src/fvm/machine.rs Outdated Show resolved Hide resolved
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

All good and straightforward now I've followed the various threads through the code. It's basically using DefaultExecutor and take over responsibility for things ThreadedExecutor is doing

@Stebalien Stebalien merged commit 5868337 into master Apr 10, 2024
8 of 9 checks passed
@Stebalien Stebalien deleted the steb/actual-concurrency branch April 10, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase LOTUS_FVM_CONCURRENCY
2 participants