Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
## 2024-11-20 - JSON Decoding Performance - Index Advancement
**Learning:** Even when avoiding string slicing using `json.JSONDecoder().raw_decode(text, index)`, failing to correctly advance the index by ignoring the returned `end` index (`value, _ = decoder.raw_decode(...)`) forces the search loop to repeatedly attempt to decode nested JSON structures (e.g., inner braces `{`) sequentially. This leads to massive O(N^2) time complexity and redundant parsing for large, deeply nested JSON objects.
**Action:** Always capture and use the new end index returned by `raw_decode` (e.g., `value, next_idx = decoder.raw_decode(text, index)`) to jump over the completely parsed object and proceed efficiently.
## 2024-06-27 - Concurrency for Subprocess I/O Wait
**Learning:** Sequential execution of subprocesses making network calls (e.g., `gh api`) inside loops causes significant O(N) I/O wait latency. Python's `subprocess.run` drops the Global Interpreter Lock (GIL), making these operations prime candidates for threading.
**Action:** Always prefer using `concurrent.futures.ThreadPoolExecutor` to execute independent subprocess I/O calls concurrently rather than relying on sequential iteration, drastically improving performance for bulk network operations.
8 changes: 8 additions & 0 deletions plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
1. **Analyze the Bottleneck:** The `enrich_rest_mergeable_states` function in `scripts/ci/pr_review_merge_scheduler.py` iterates over a list of PRs and sequentially calls `fetch_rest_mergeable_state(repo, int(pr["number"]))` for each. This function issues a network call via `subprocess.run(["gh", "api", ...])`. In a loop, this leads to O(N) I/O wait latency, which is slow for a large number of PRs.
2. **Proposed Optimization:** Use `concurrent.futures.ThreadPoolExecutor` to run these subprocesses concurrently. As the memory hint states, `subprocess.run` drops the Python GIL, making it an excellent candidate for threading.
3. **Refactoring Details:**
* Import `concurrent.futures`.
* Update `enrich_rest_mergeable_states` to execute the `fetch_rest_mergeable_state` calls concurrently using `ThreadPoolExecutor.map` or submitting jobs.
Comment thread
github-actions[bot] marked this conversation as resolved.
Resolved
* Maintain the existing error handling (catching `RuntimeError`) and mapping of the result to the correct PR.
4. **Verification:** Run the tests `PYTHONPATH=$(pwd) pytest --cov=scripts/ci tests/` to ensure no functionality is broken and we retain 100% test coverage.
5. **Pre-commit & Submit:** Add comments, run any formatting/linting scripts, perform pre-commit instructions, and submit the changes.
11 changes: 9 additions & 2 deletions scripts/ci/pr_review_merge_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import annotations

import argparse
import concurrent.futures
import json
import os
import re
Expand Down Expand Up @@ -407,12 +408,18 @@ def fetch_rest_mergeable_state(repo: str, number: int) -> str:

def enrich_rest_mergeable_states(repo: str, prs: list[dict[str, Any]]) -> None:
"""Attach REST mergeability evidence to GraphQL pull request payloads."""
for pr in prs:
def enrich_pr(pr: dict[str, Any]) -> None:
"""Fetch and attach state for a single PR."""
try:
pr["restMergeableState"] = fetch_rest_mergeable_state(repo, int(pr["number"]))
state = fetch_rest_mergeable_state(repo, int(pr["number"]))
pr["restMergeableState"] = state
except RuntimeError as exc:
pr["restMergeableStateError"] = bounded_error_summary(str(exc))

# ⚡ Bolt: Execute independent subprocess network calls concurrently to avoid O(N) I/O wait latency
with concurrent.futures.ThreadPoolExecutor() as executor:
list(executor.map(enrich_pr, prs))
Comment thread
github-actions[bot] marked this conversation as resolved.
Resolved


def effective_merge_state(pr: dict[str, Any]) -> str:
"""Return the safest merge state from GraphQL plus REST mergeability evidence."""
Expand Down
Loading