Repository navigation
ci: add paired Python SDK performance benchmarks - #1134
dustinbyrne wants to merge 2 commits into
Conversation
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
Re-add the stamphog label to request another review once you have addressed this.
stamphog declined to auto-review this pull request because three gates failed. The deny-list gate matched dependency/toolchain files (benchmarks/requirements.in and benchmarks/requirements.txt) and CI/CD infrastructure (.github/workflows/benchmarks.yml). The size gate failed because the change has 1289 substantive lines against an 800-line ceiling. The tier gate classified it as T2-never because it spans two areas and touches CI.
Please ask a human reviewer to look at it. If you want it to be reviewable on its own, consider splitting it so the workflow and dependency lockfile changes land separately from the benchmark code, with each part under the size ceiling.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: deps_toolchain, infra_cicd |
| size | ✗ | too large for auto-review (1289L substantive in global — ceiling is 800L; 1289L, 11F total, 2043L/14F incl. docs/generated/snapshots) |
| tier | ✗ | classified as T2-never: T2-never (2043L, 14F, two-areas, ci) |
| stamphog 2.6.1 | .stamphog/policy.yml @ unknown · reviewed head 50dff4b |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 50dff4b: 2 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
| with subprocess.Popen( | ||
| command, | ||
| cwd=directory, | ||
| env=worker_env(), |
There was a problem hiding this comment.
P2 · Keep profiler traces inside the coordinator-owned temporary directory
When an allocation worker exceeds its timeout or RSS limit, process.kill() bypasses cleanup of sample_allocations()'s temporary directory. run_worker() removes only its separate result directory, leaving large Memray traces behind. Repeated failures accumulate disk usage on local and self-hosted runners; set the child's TMPDIR to the coordinator-owned directory.
| if any(record["status"] != "ok" for record in records.values()): | ||
| break |
There was a problem hiding this comment.
P2 · Bound cumulative time spent on failed workers
When a flag-evaluation regression hangs each head worker, this break skips only remaining repetitions and continues through every workload and pass. The nine single-flag workloads alone consume 36 minutes of head-worker timeouts, exceeding the workflow's 30-minute budget before later workloads run; enforce a suite deadline or stop after repeated timeouts.
posthog-python Compliance ReportDate: 2026-10-27T21:56:30.406425+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
Motivation and context
Add reproducible PR-local measurements of Python SDK flag evaluation, allocation churn, memory under load/refresh, and capture/drain. This is the Python producer for the same provisional JSON contract as Node.
Changes
Shared report rendering and commenting will integrate in a stacked follow-up. No SDK runtime changes or release required.
How did you test it?
Checklist
Agent context
Autonomy: Human-driven (agent-assisted).
Implemented with Pi and a fresh read-only reviewer. Allocator calls are explicitly not object counts; unsupported uninstrumented heap bytes and enqueue-only allocations remain visible rather than represented as zero.