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 CLI arg to control size of rayon global thread pool #3392

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

steviez
Copy link

@steviez steviez commented Oct 30, 2024

Problem

#105

Summary of Changes

The arg is hidden for now, and default behavior of one thread in the pool per system thread remains

To test, I:

  • Made note of number of threads before this machine (64 for my box)
  • Restarted a node with this commit (without flag set) and observed the pool is same size
  • Restart a node with the commit + flag set and observed custom size is respected

The arg is hidden for now, and default behavior of one thread in the
pool per system thread remains
@steviez steviez force-pushed the val_cli_rayon_global branch from 3c32b45 to aa20087 Compare October 30, 2024 20:30
@steviez steviez requested a review from bw-solana October 30, 2024 20:45
Copy link

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

LGTM. One thing to consider

core/src/validator.rs Show resolved Hide resolved

fn default() -> usize {
get_max_thread_count()
}

Choose a reason for hiding this comment

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

Is 1 (the default) okay for the min here or should we bump it?

Copy link
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 ok - if someone specifies a bad value for a hidden argument that is on them IMO

On a somewhat related note, I think trying to get most operations (if not everything) out of the global pool is a good goal; the global pool is easy to use but given that anyone other service could be lighting it up with work, not great to have that uncertainty.

@steviez steviez merged commit 4994762 into anza-xyz:master Oct 30, 2024
40 checks passed
@steviez steviez deleted the val_cli_rayon_global branch October 30, 2024 23:16
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
The arg is hidden for now, and default behavior of one thread in the
pool per system thread remains
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants