-
Notifications
You must be signed in to change notification settings - Fork 0
Introduce benchmark CI and Python binding test suite #45
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
6ade8e4
Introduce label-gated benchmark CI workflow for PR comparison
HudsonGraeme 2e4d482
Introduce Python binding test suite and CI job
HudsonGraeme e37aed0
Raise benchmark regression threshold to 10% for shared runner noise t…
HudsonGraeme bb02ad9
Resolve config validation failures in custom config constructor tests
HudsonGraeme 8824bda
Resolve resource leaks and imprecise exception assertions in Python t…
HudsonGraeme f97b0c3
Resolve missing cleanup guarantee in query_without_signer error test
HudsonGraeme File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| name: Benchmarks | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [labeled, synchronize] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| jobs: | ||
| bench: | ||
| name: Benchmark comparison | ||
| runs-on: ubuntu-latest | ||
| if: contains(github.event.pull_request.labels.*.name, 'bench') | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 | ||
| with: | ||
| toolchain: stable | ||
|
|
||
| - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 | ||
|
|
||
| - name: Build benchmark (PR) | ||
| run: cargo build --release -p lightning-bench | ||
|
|
||
| - name: Copy PR binary | ||
| run: cp target/release/lightning-bench /tmp/lightning-bench-pr | ||
|
|
||
| - name: Checkout main | ||
| run: git checkout origin/main | ||
|
|
||
| - name: Build benchmark (main) | ||
| id: build-main | ||
| continue-on-error: true | ||
| run: cargo build --release -p lightning-bench | ||
|
|
||
| - name: Copy main binary | ||
| if: steps.build-main.outcome == 'success' | ||
| run: cp target/release/lightning-bench /tmp/lightning-bench-main | ||
|
|
||
| - name: Run main benchmark | ||
| if: steps.build-main.outcome == 'success' | ||
| run: /tmp/lightning-bench-main > /tmp/bench-main.json 2>/dev/null | ||
|
|
||
| - name: Run PR benchmark | ||
| run: /tmp/lightning-bench-pr > /tmp/bench-pr.json 2>/dev/null | ||
|
|
||
| - name: Post comparison comment | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7 | ||
| with: | ||
| script: | | ||
| const fs = require('fs'); | ||
| const pr = JSON.parse(fs.readFileSync('/tmp/bench-pr.json', 'utf8')); | ||
|
|
||
| let hasMain = false; | ||
| let main = {}; | ||
| try { | ||
| main = JSON.parse(fs.readFileSync('/tmp/bench-main.json', 'utf8')); | ||
| hasMain = true; | ||
| } catch {} | ||
|
|
||
| function fmt(v) { | ||
| return typeof v === 'number' ? v.toFixed(2) : String(v); | ||
| } | ||
|
|
||
| function pctChange(oldVal, newVal, lowerIsBetter) { | ||
| if (!oldVal || oldVal === 0) return ''; | ||
| const pct = ((newVal - oldVal) / oldVal) * 100; | ||
| const sign = pct > 0 ? '+' : ''; | ||
| const tag = lowerIsBetter | ||
| ? (pct > 15 ? ' !!!' : pct < -15 ? ' +++' : '') | ||
| : (pct < -15 ? ' !!!' : pct > 15 ? ' +++' : ''); | ||
| return ` (${sign}${pct.toFixed(1)}%${tag})`; | ||
| } | ||
|
|
||
| let body = '<!-- bench-results -->\n## Benchmark Results\n\n'; | ||
|
|
||
| body += '### Connection Setup (ms)\n\n'; | ||
| body += '| Percentile | PR |' + (hasMain ? ' main | change |' : '') + '\n'; | ||
| body += '|---|---|' + (hasMain ? '---|---|' : '') + '\n'; | ||
| for (const p of ['p50', 'p95', 'p99']) { | ||
| const prVal = pr.connection_setup_ms[p]; | ||
| if (hasMain) { | ||
| const mainVal = main.connection_setup_ms[p]; | ||
| body += `| ${p} | ${fmt(prVal)} | ${fmt(mainVal)} | ${pctChange(mainVal, prVal, true)} |\n`; | ||
| } else { | ||
| body += `| ${p} | ${fmt(prVal)} |\n`; | ||
| } | ||
| } | ||
|
|
||
| const sizes = Object.keys(pr.latency_ms).sort((a, b) => { | ||
| const order = {'256B': 0, '1KB': 1, '10KB': 2, '100KB': 3, '1MB': 4}; | ||
| return (order[a] ?? 99) - (order[b] ?? 99); | ||
| }); | ||
|
|
||
| body += '\n### Latency (ms)\n\n'; | ||
| body += '| Size | Percentile | PR |' + (hasMain ? ' main | change |' : '') + '\n'; | ||
| body += '|---|---|---|' + (hasMain ? '---|---|' : '') + '\n'; | ||
| for (const size of sizes) { | ||
| for (const p of ['p50', 'p95', 'p99']) { | ||
| const prVal = pr.latency_ms[size][p]; | ||
| if (hasMain && main.latency_ms?.[size]) { | ||
| const mainVal = main.latency_ms[size][p]; | ||
| body += `| ${size} | ${p} | ${fmt(prVal)} | ${fmt(mainVal)} | ${pctChange(mainVal, prVal, true)} |\n`; | ||
| } else { | ||
| body += `| ${size} | ${p} | ${fmt(prVal)} |\n`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| body += '\n### Throughput (req/s)\n\n'; | ||
| body += '| Size | PR |' + (hasMain ? ' main | change |' : '') + '\n'; | ||
| body += '|---|---|' + (hasMain ? '---|---|' : '') + '\n'; | ||
| for (const size of sizes) { | ||
| const prVal = pr.throughput_rps[size]; | ||
| if (hasMain && main.throughput_rps?.[size] != null) { | ||
| const mainVal = main.throughput_rps[size]; | ||
| body += `| ${size} | ${fmt(prVal)} | ${fmt(mainVal)} | ${pctChange(mainVal, prVal, false)} |\n`; | ||
| } else { | ||
| body += `| ${size} | ${fmt(prVal)} |\n`; | ||
| } | ||
| } | ||
|
|
||
| if (pr.wire_bytes) { | ||
| body += '\n### Wire Bytes\n\n'; | ||
| body += '| Size | PR |' + (hasMain ? ' main | change |' : '') + '\n'; | ||
| body += '|---|---|' + (hasMain ? '---|---|' : '') + '\n'; | ||
| for (const size of sizes) { | ||
| const prVal = pr.wire_bytes[size]; | ||
| if (hasMain && main.wire_bytes?.[size] != null) { | ||
| const mainVal = main.wire_bytes[size]; | ||
| body += `| ${size} | ${prVal} | ${mainVal} | ${pctChange(mainVal, prVal, true)} |\n`; | ||
| } else { | ||
| body += `| ${size} | ${prVal} |\n`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const marker = '<!-- bench-results -->'; | ||
| const { data: comments } = await github.rest.issues.listComments({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| }); | ||
| const existing = comments.find(c => c.body?.includes(marker)); | ||
|
|
||
| if (existing) { | ||
| await github.rest.issues.updateComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| comment_id: existing.id, | ||
| body, | ||
| }); | ||
| } else { | ||
| await github.rest.issues.createComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| body, | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import socket | ||
| import threading | ||
| import time | ||
|
|
||
| import pytest | ||
|
|
||
| from btlightning import Lightning, LightningServer | ||
|
|
||
| MINER_SEED = bytes([1] * 32) | ||
| VALIDATOR_SEED = bytes([2] * 32) | ||
| MINER_HOTKEY = "5CcyqxXnJucaCnQQvvUg5EPzj1uoNAxACZvzArHw5aVDvgNH" | ||
| VALIDATOR_HOTKEY = "5CfCr47V5Dte6bwxNBE8K9oNnQd9fiay6aDEEkgYtFv7w4Fq" | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def free_port(): | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("127.0.0.1", 0)) | ||
| return s.getsockname()[1] | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def echo_server(free_port): | ||
| server = LightningServer(miner_hotkey=MINER_HOTKEY, host="127.0.0.1", port=free_port) | ||
| server.set_miner_keypair(MINER_SEED) | ||
| server.register_synapse_handler("echo", lambda data: data) | ||
| server.start() | ||
| t = threading.Thread(target=server.serve_forever, daemon=True) | ||
| t.start() | ||
| time.sleep(0.05) | ||
| yield server, free_port | ||
| server.stop() | ||
| t.join(timeout=5) | ||
|
|
||
|
Comment on lines
+27
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Harden fixture lifecycle to reduce startup flakiness and thread leakage. A fixed delay is brittle, and teardown should wait for the serve thread to exit after ♻️ Proposed fixture hardening def echo_server(free_port):
@@
server.register_synapse_handler("echo", lambda data: data)
server.start()
t = threading.Thread(target=server.serve_forever, daemon=True)
t.start()
- time.sleep(0.05)
+ deadline = time.time() + 2.0
+ while time.time() < deadline:
+ try:
+ with socket.create_connection(("127.0.0.1", free_port), timeout=0.1):
+ break
+ except OSError:
+ time.sleep(0.01)
+ else:
+ raise AssertionError("echo server did not become ready in time")
yield server, free_port
server.stop()
+ t.join(timeout=1.0)🤖 Prompt for AI Agents |
||
|
|
||
| @pytest.fixture() | ||
| def client_and_axon(echo_server): | ||
| _, port = echo_server | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| client.set_validator_keypair(VALIDATOR_SEED) | ||
| axon = {"hotkey": MINER_HOTKEY, "ip": "127.0.0.1", "port": port} | ||
| client.initialize_connections([axon]) | ||
| yield client, axon | ||
| client.close() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| from btlightning import Lightning | ||
|
|
||
| from conftest import VALIDATOR_HOTKEY, VALIDATOR_SEED | ||
|
|
||
|
|
||
| def test_constructor_defaults(): | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| assert client.wallet_hotkey == VALIDATOR_HOTKEY | ||
| client.close() | ||
|
|
||
|
|
||
| def test_constructor_custom_config(): | ||
| client = Lightning( | ||
| wallet_hotkey=VALIDATOR_HOTKEY, | ||
| connect_timeout_secs=5, | ||
| idle_timeout_secs=30, | ||
| keep_alive_interval_secs=10, | ||
| reconnect_initial_backoff_secs=1, | ||
| reconnect_max_backoff_secs=60, | ||
| reconnect_max_retries=3, | ||
| max_frame_payload_bytes=2097152, | ||
| max_stream_payload_bytes=10485760, | ||
| ) | ||
| assert client.wallet_hotkey == VALIDATOR_HOTKEY | ||
| client.close() | ||
|
|
||
|
|
||
| def test_set_validator_keypair(): | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| client.set_validator_keypair(VALIDATOR_SEED) | ||
| client.close() | ||
|
|
||
|
|
||
| def test_set_python_signer(): | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| client.set_python_signer(lambda msg: b"\x00" * 64) | ||
| client.close() | ||
|
|
||
|
|
||
| def test_get_connection_stats_empty(): | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| stats = client.get_connection_stats() | ||
| assert "total_connections" in stats | ||
| assert "active_miners" in stats | ||
| client.close() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import pytest | ||
|
|
||
| from btlightning import Lightning | ||
|
|
||
| from conftest import MINER_HOTKEY, VALIDATOR_HOTKEY | ||
|
|
||
|
|
||
| def test_missing_synapse_type(client_and_axon): | ||
| client, axon = client_and_axon | ||
| with pytest.raises(KeyError, match="synapse_type"): | ||
| client.query_axon(axon, {"data": {"msg": "hello"}}) | ||
|
|
||
|
|
||
| def test_missing_hotkey(client_and_axon): | ||
| client, _ = client_and_axon | ||
| with pytest.raises(KeyError, match="hotkey"): | ||
| client.query_axon({"ip": "127.0.0.1", "port": 1234}, {"synapse_type": "echo", "data": {}}) | ||
|
|
||
|
|
||
| def test_missing_ip(client_and_axon): | ||
| client, _ = client_and_axon | ||
| with pytest.raises(KeyError, match="ip"): | ||
| client.query_axon({"hotkey": MINER_HOTKEY, "port": 1234}, {"synapse_type": "echo", "data": {}}) | ||
|
|
||
|
|
||
| def test_missing_port(client_and_axon): | ||
| client, _ = client_and_axon | ||
| with pytest.raises(KeyError, match="port"): | ||
| client.query_axon({"hotkey": MINER_HOTKEY, "ip": "127.0.0.1"}, {"synapse_type": "echo", "data": {}}) | ||
|
|
||
|
|
||
| def test_query_without_signer(): | ||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| axon = {"hotkey": MINER_HOTKEY, "ip": "127.0.0.1", "port": 9999} | ||
| with pytest.raises(ConnectionError, match="endpoint not initialized"): | ||
| client.query_axon(axon, {"synapse_type": "echo", "data": {}}) | ||
| client.close() | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
|
|
||
| def test_invalid_timeout(client_and_axon): | ||
| client, axon = client_and_axon | ||
| with pytest.raises(ValueError, match="timeout_secs must be a finite positive number"): | ||
| client.query_axon(axon, {"synapse_type": "echo", "data": {}}, timeout_secs=-1.0) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import socket | ||
| import threading | ||
| import time | ||
|
|
||
| from btlightning import Lightning, LightningServer | ||
|
|
||
| from conftest import MINER_HOTKEY, MINER_SEED, VALIDATOR_HOTKEY, VALIDATOR_SEED | ||
|
|
||
|
|
||
| def test_two_handlers_on_same_server(): | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("127.0.0.1", 0)) | ||
| port = s.getsockname()[1] | ||
|
|
||
| def upper_handler(data): | ||
| return {"result": data["text"].upper()} | ||
|
|
||
| server = LightningServer(miner_hotkey=MINER_HOTKEY, host="127.0.0.1", port=port) | ||
| server.set_miner_keypair(MINER_SEED) | ||
| server.register_synapse_handler("echo", lambda data: data) | ||
| server.register_synapse_handler("upper", upper_handler) | ||
| server.start() | ||
| t = threading.Thread(target=server.serve_forever, daemon=True) | ||
| t.start() | ||
| time.sleep(0.05) | ||
|
|
||
| client = Lightning(wallet_hotkey=VALIDATOR_HOTKEY) | ||
| client.set_validator_keypair(VALIDATOR_SEED) | ||
| axon = {"hotkey": MINER_HOTKEY, "ip": "127.0.0.1", "port": port} | ||
| client.initialize_connections([axon]) | ||
|
|
||
| try: | ||
| echo_resp = client.query_axon(axon, {"synapse_type": "echo", "data": {"msg": "hi"}}) | ||
| assert echo_resp["success"] is True | ||
| assert echo_resp["data"]["msg"] == "hi" | ||
|
|
||
| upper_resp = client.query_axon(axon, {"synapse_type": "upper", "data": {"text": "hello"}}) | ||
| assert upper_resp["success"] is True | ||
| assert upper_resp["data"]["result"] == "HELLO" | ||
| finally: | ||
| client.close() | ||
| server.stop() | ||
| t.join(timeout=5) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression/improvement threshold appears inconsistent with the stated 10% tolerance.
The benchmark marker logic is still using
15%, so comment annotations won’t match the intended tolerance.🛠️ Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents