Remove Piccolo performance framework - #8237
Conversation
e1838a4 to
5359435
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the Piccolo performance framework and its associated tooling (Python modules, native submit client, CMake integration, docs, and related dependencies), while keeping the remaining basic_blocking_locust perf coverage and migrating remaining plotting utilities away from pandas/Arrow/Parquet.
Changes:
- Removed Piccolo’s Python framework, the C++
submitter and its CCF client dependencies, plus all associated CMake wiring and documentation. - Simplified the Basic perf coverage to the blocking Locust benchmark and updated docs/config accordingly.
- Migrated remaining CSV/plotting paths to Polars (and replaced scikit-learn linear regression with numpy).
Custom instructions used:
.github/copilot-instructions.md.github/instructions/changelog.instructions.md.github/instructions/reviewing.instructions.md
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/requirements.txt | Drops Piccolo-only Python deps from the test environment. |
| tests/perf-system/submitter/submit.cpp | Removes the native Parquet-based submitter client. |
| tests/perf-system/submitter/parquet_data.h | Removes submitter Parquet data container. |
| tests/perf-system/submitter/handle_arguments.h | Removes submitter CLI argument parser. |
| tests/perf-system/submitter/CMakeLists.txt | Removes submitter build target and link deps. |
| tests/infra/service_load.py | Switches Locust results plotting from pandas to Polars. |
| tests/infra/remote_client.py | Removes Piccolo-specific remote-client invocation path. |
| tests/infra/piccolo/throughput_analysis.py | Deletes Piccolo throughput analysis script. |
| tests/infra/piccolo/logging_generator.py | Deletes Piccolo logging workload generator. |
| tests/infra/piccolo/generator.py | Deletes Piccolo request generator library. |
| tests/infra/piccolo/generate_packages.py | Deletes Piccolo generator CLI wrapper. |
| tests/infra/piccolo/analyzer.py | Deletes Piccolo analysis library. |
| tests/infra/piccolo/analyze_packages.py | Deletes Piccolo analysis CLI wrapper. |
| tests/infra/piccolo/init.py | Removes Piccolo package marker. |
| tests/infra/piccolo_driver.py | Deletes Piccolo driver benchmark orchestration. |
| tests/infra/perf.py | Removes Piccolo perf CSV header constant module. |
| tests/infra/basicperf.py | Deletes non-blocking Basic benchmark and its tooling usage. |
| tests/infra/basicperf_locustfile.py | Narrows the Locust workload to blocking writes only. |
| tests/basicperf_locust.py | Updates the perf test harness to only run blocking workload. |
| src/clients/tls_client.h | Removes TLS client used by the submitter path. |
| src/clients/rpc_tls_client.h | Removes RPC-over-TLS client used by submitter/perf tooling. |
| src/clients/perf/perf_client.h | Removes perf client option plumbing for removed benchmarks. |
| scripts/setup-ci.sh | Drops Arrow/Parquet build deps from CI environment. |
| scripts/setup-ci-al4.sh | Drops Arrow/Parquet build deps from AL4 CI environment. |
| scripts/ascii-checks.sh | Removes allowlist entry for deleted perf script. |
| samples/scripts/plot_tx_times.py | Migrates CSV handling to Polars and drops scikit-learn. |
| doc/overview/performance.rst | Updates perf docs to reference the remaining benchmark. |
| doc/architecture/performance/submitter.rst | Removes submitter documentation page. |
| doc/architecture/performance/index.rst | Removes Piccolo architecture landing page. |
| doc/architecture/performance/generator.rst | Removes generator documentation page. |
| doc/architecture/performance/analysis.rst | Removes analysis documentation page. |
| doc/architecture/index.rst | Removes the architecture entrypoint link to Piccolo docs. |
| CMakeLists.txt | Removes Piccolo perf tests and submitter build inclusion. |
| cmake/gersemi_definitions.cmake | Removes the add_piccolo_test gersemi shim. |
| cmake/common.cmake | Removes the add_piccolo_test helper implementation. |
| CHANGELOG.md | Documents removal of Piccolo tooling and benchmarks. |
| .gitignore | Removes ignores for Piccolo-generated Parquet/PNG artifacts. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if __name__ == "__main__": | ||
| args = cli_args() | ||
| # The workload targets the primary, and additional nodes only add | ||
| # replication cost. | ||
| # A single node is enough: the benchmark measures the time taken to commit | ||
| # on the primary, and additional nodes only add replication cost. | ||
| args.nodes = infra.e2e_args.min_nodes(args, f=0) |
5359435 to
0e45125
Compare
|
|
||
| ### Removed | ||
|
|
||
| - The Piccolo performance framework, its `submit` client, and its non-blocking Basic performance benchmarks have been removed. The `basic_blocking_locust` benchmark remains for Basic end-to-end performance coverage. (#8237) |
There was a problem hiding this comment.
Isn't it pure infra? We never exported it, and there's no mention of adding/changing piccolo in the changelog before.
There was a problem hiding this comment.
Good point
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
0e45125 to
b21d770
Compare
Summary
basic_blocking_locustsubmitclient, orphaned client support, documentation, and CMake wiringValidation
PUT /records/blocking/{key}--js-app-bundleis rejected with a clear explanation that the benchmark requires the C++-only blocking endpoint