feat(concurrency): implement Default for ConcurrencyConfig

[barak-b-starkware]

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.17%. Comparing base (702f401) to head (cf5a729).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1952   +/-   ##
=======================================
  Coverage   78.17%   78.17%           
=======================================
  Files          62       62           
  Lines        8842     8842           
  Branches     8842     8842           
=======================================
  Hits         6912     6912           
  Misses       1492     1492           
  Partials      438      438           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[avi-starkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @barak-b-starkware, @MohammadNassar1, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/blockifier/config.rs line 15 at r1 (raw file):

impl Default for ConcurrencyConfig {
    fn default() -> Self {
        Self { enabled: false, n_workers: 64, chunk_size: 4 }

n_workers and chunk_size here have swapped values from the default consts in utils.rs.

Why don't you use the constants here?

Suggestion:

        Self { enabled: false, n_workers: 4, chunk_size: 64 }

---

crates/blockifier/src/concurrency/utils.rs line 15 at r1 (raw file):

pub const DEFAULT_CHUNK_SIZE: usize = 64;
pub const DEFAULT_N_WORKERS: usize = 4;

Ittay Dror said we should determine the number of workers automatically based on the actual hardware we are running on. Otherwise, we would have to reconfigure the batcher each time we change the machine it is running on.

This is probably not in the scope of this PR, but maybe add a TODO somewhere?

Nonblocking.

Code quote:

pub const DEFAULT_N_WORKERS: usize = 4;

[barak-b-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @MohammadNassar1, @noaov1, @OriStarkware, and @Yoni-Starkware)

---

crates/blockifier/src/blockifier/config.rs line 15 at r1 (raw file):

Previously, avi-starkware wrote…

n_workers and chunk_size here have swapped values from the default consts in utils.rs.

Why don't you use the constants here?

Done.
To use constants from the concurrency module, we need to find some workaround since the concurrency module can be seen only when adding --feartures=concurrency.
I think we can remove the #[cfg(feature = concurrency)].

---

crates/blockifier/src/concurrency/utils.rs line 15 at r1 (raw file):

Previously, avi-starkware wrote…

Ittay Dror said we should determine the number of workers automatically based on the actual hardware we are running on. Otherwise, we would have to reconfigure the batcher each time we change the machine it is running on.

This is probably not in the scope of this PR, but maybe add a TODO somewhere?

Nonblocking.

Removed from this PR.

[avi-starkware]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MohammadNassar1, @noaov1, @OriStarkware, and @Yoni-Starkware)

[Yoni-Starkware]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MohammadNassar1, @noaov1, and @OriStarkware)

The base branch was changed.