Skip to content

Conversation

@Oppen
Copy link
Contributor

@Oppen Oppen commented Nov 11, 2025

Motivation

Rayon currently inherits the name of the thread that calls it first, which creates
confusion when, for example, one tries to see which threads incur the most contention.

Description

Initialize the global thread pool on init to ensure the Rayon workers each have a
unique name.

@Oppen Oppen requested a review from a team as a code owner November 11, 2025 14:44
Copilot AI review requested due to automatic review settings November 11, 2025 14:44
@github-actions github-actions bot added L1 Ethereum client L2 Rollup client labels Nov 11, 2025
Copilot finished reviewing on behalf of Oppen November 11, 2025 14:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds unique naming to Rayon worker threads to facilitate debugging and performance analysis. Previously, Rayon threads inherited the name of the calling thread, making it difficult to identify which threads were experiencing contention.

  • Initializes the global Rayon thread pool with custom thread names
  • Adds rayon dependency to the ethrex binary crate

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
cmd/ethrex/ethrex.rs Adds Rayon thread pool initialization with custom naming pattern in the main function
cmd/ethrex/Cargo.toml Adds rayon workspace dependency to the ethrex binary crate
Cargo.lock Updates dependency graph to include rayon for the ethrex binary

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rayon::ThreadPoolBuilder::default()
.thread_name(|i| format!("rayon-worker-{i}"))
.build_global()
.expect("failed to build rayon threadpool");
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The error message uses 'threadpool' but it should be 'thread pool' (two words) for consistency with the standard terminology.

Suggested change
.expect("failed to build rayon threadpool");
.expect("failed to build rayon thread pool");

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Lines of code report

Total lines added: 4
Total lines removed: 0
Total lines changed: 4

Detailed view
+-----------------------------+-------+------+
| File                        | Lines | Diff |
+-----------------------------+-------+------+
| ethrex/cmd/ethrex/ethrex.rs | 66    | +4   |
+-----------------------------+-------+------+

@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Nov 11, 2025
@jrchatruc jrchatruc enabled auto-merge November 11, 2025 14:49
@jrchatruc jrchatruc added this pull request to the merge queue Nov 11, 2025
Merged via the queue into main with commit ce98532 Nov 11, 2025
50 checks passed
@jrchatruc jrchatruc deleted the feat/name_rayon_threads branch November 11, 2025 15:40
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 11, 2025
xqft pushed a commit that referenced this pull request Nov 11, 2025
**Motivation**

Rayon currently inherits the name of the thread that calls it first,
which creates
confusion when, for example, one tries to see which threads incur the
most contention.

**Description**

Initialize the global thread pool on init to ensure the Rayon workers
each have a
unique name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client L2 Rollup client

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants