perf: parallelize rule execution using ThreadPoolExecutor#96
Conversation
📝 WalkthroughWalkthroughThe evaluate and evaluate_message functions in src/becwright/engine.py are refactored to execute rule checks concurrently using ThreadPoolExecutor instead of sequentially, submitting _run_check tasks and gathering results from futures before returning Result(per_rule=results). ChangesParallel Rule Execution
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Engine
participant ThreadPoolExecutor
participant RunCheck
Caller->>Engine: evaluate(rules, stdin, root)
Engine->>Engine: build tasks (rule, stdin) for matching files rules
Engine->>ThreadPoolExecutor: submit RunCheck for each task
loop for each task
ThreadPoolExecutor->>RunCheck: run(rule, stdin, root)
RunCheck-->>ThreadPoolExecutor: RuleResult
end
ThreadPoolExecutor-->>Engine: collected results
Engine-->>Caller: Result(per_rule=results)
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/becwright/engine.py (1)
107-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared ThreadPoolExecutor submit-and-gather pattern.
Both
evaluateandevaluate_messageduplicate the samewith ThreadPoolExecutor()→executor.submit→f.result()flow. Extracting a small helper eliminates the duplication and ensures both paths stay consistent if error handling or worker limits change later.Additionally,
taskshas different semantics in each function:list[tuple[Rule, str]]inevaluatevslist[Rule]inevaluate_message. Normalizingevaluate_messageto build(rule, message_path)tuples would make both call sites identical.♻️ Proposed helper extraction
def _run_checks(tasks: list[tuple[Rule, str]], root: Path) -> list[RuleResult]: with ThreadPoolExecutor() as executor: futures = [ executor.submit(_run_check, rule, stdin, root) for rule, stdin in tasks ] return [f.result() for f in futures]Then in
evaluate:- with ThreadPoolExecutor() as executor: - futures = [ - executor.submit(_run_check, rule, stdin, root) - for rule, stdin in tasks - ] - results = [f.result() for f in futures] - return Result(per_rule=results) + return Result(per_rule=_run_checks(tasks, root))And in
evaluate_message:- tasks = [ - rule for rule in rules - if rule.target == "commit-msg" - ] - with ThreadPoolExecutor() as executor: - futures = [ - executor.submit(_run_check, rule, message_path, root) - for rule in tasks - ] - results = [f.result() for f in futures] - return Result(per_rule=results) + tasks = [ + (rule, message_path) for rule in rules + if rule.target == "commit-msg" + ] + return Result(per_rule=_run_checks(tasks, root))Also applies to: 126-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/becwright/engine.py` around lines 107 - 115, The `evaluate` and `evaluate_message` paths both repeat the same ThreadPoolExecutor submit-and-gather pattern; extract that shared logic into a small helper so both call sites use the same execution flow. While doing so, normalize `evaluate_message` to build the same `(rule, input)` task tuples as `evaluate`, and have the helper accept the common task shape and return the collected results consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/becwright/engine.py`:
- Around line 107-115: The `evaluate` and `evaluate_message` paths both repeat
the same ThreadPoolExecutor submit-and-gather pattern; extract that shared logic
into a small helper so both call sites use the same execution flow. While doing
so, normalize `evaluate_message` to build the same `(rule, input)` task tuples
as `evaluate`, and have the helper accept the common task shape and return the
collected results consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 324bb5ae-0d1c-4814-929c-ff2ecc900744
📒 Files selected for processing (1)
src/becwright/engine.py
Description
This PR parallelizes the execution of rule checks in
becwright. Instead of running rules sequentially in a loop, it uses Python's built-inconcurrent.futures.ThreadPoolExecutorto run check subprocesses concurrently.Since checking tasks are independent and spend almost all of their time blocked on I/O (spawning and waiting on
subprocess.run), executing them in a thread pool yields significant performance gains. The total check duration is now bound by the single slowest check, rather than the sum of all checks combined.Fixes #95
Key Changes
1. Concurrency Implementation
src/becwright/engine.py:evaluate()to aggregate matched files for each file-targeted rule, then queue and execute them concurrently viaThreadPoolExecutor.evaluate_message()similarly to run commit message rules concurrently.Verification Results
The test suite runs fully green:
Summary by CodeRabbit