chore(amber): remove the unused ElidableStatement - #7451
Conversation
Automated Reviewer SuggestionsBased on the
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 429 | 0.262 | 22,854/31,269/31,269 us | 🔴 +22.1% / 🔴 +94.0% |
| 🟢 | bs=100 sw=10 sl=64 | 948 | 0.578 | 104,313/125,771/125,771 us | 🟢 -17.2% / 🔴 +13.1% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,110 | 0.677 | 895,992/984,290/984,290 us | ⚪ within ±5% / 🟢 +12.8% |
Baseline details
Latest main e03d971 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 429 tuples/sec | 442 tuples/sec | 752.25 tuples/sec | -2.9% | -43.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.262 MB/s | 0.27 MB/s | 0.459 MB/s | -3.0% | -42.9% |
| bs=10 sw=10 sl=64 | p50 | 22,854 us | 18,712 us | 13,006 us | +22.1% | +75.7% |
| bs=10 sw=10 sl=64 | p95 | 31,269 us | 37,382 us | 16,117 us | -16.4% | +94.0% |
| bs=10 sw=10 sl=64 | p99 | 31,269 us | 37,382 us | 19,286 us | -16.4% | +62.1% |
| bs=100 sw=10 sl=64 | throughput | 948 tuples/sec | 973 tuples/sec | 953.66 tuples/sec | -2.6% | -0.6% |
| bs=100 sw=10 sl=64 | MB/s | 0.578 MB/s | 0.594 MB/s | 0.582 MB/s | -2.7% | -0.7% |
| bs=100 sw=10 sl=64 | p50 | 104,313 us | 99,986 us | 105,130 us | +4.3% | -0.8% |
| bs=100 sw=10 sl=64 | p95 | 125,771 us | 151,855 us | 111,226 us | -17.2% | +13.1% |
| bs=100 sw=10 sl=64 | p99 | 125,771 us | 151,855 us | 121,065 us | -17.2% | +3.9% |
| bs=1000 sw=10 sl=64 | throughput | 1,110 tuples/sec | 1,140 tuples/sec | 984.25 tuples/sec | -2.6% | +12.8% |
| bs=1000 sw=10 sl=64 | MB/s | 0.677 MB/s | 0.696 MB/s | 0.601 MB/s | -2.7% | +12.7% |
| bs=1000 sw=10 sl=64 | p50 | 895,992 us | 868,196 us | 1,024,295 us | +3.2% | -12.5% |
| bs=1000 sw=10 sl=64 | p95 | 984,290 us | 945,887 us | 1,062,710 us | +4.1% | -7.4% |
| bs=1000 sw=10 sl=64 | p99 | 984,290 us | 945,887 us | 1,095,400 us | +4.1% | -10.1% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,465.99,200,128000,429,0.262,22853.77,31269.09,31269.09
1,100,10,64,20,2110.69,2000,1280000,948,0.578,104312.74,125770.80,125770.80
2,1000,10,64,20,18020.52,20000,12800000,1110,0.677,895991.78,984290.20,984290.20
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7451 +/- ##
============================================
- Coverage 85.32% 85.32% -0.01%
+ Complexity 4151 4149 -2
============================================
Files 1169 1168 -1
Lines 46740 46739 -1
Branches 5202 5202
============================================
- Hits 39882 39880 -2
+ Misses 5148 5147 -1
- Partials 1710 1712 +2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🔴 1 must-fix · 1 advisory · 0 polish — the deletion is correct and well-scoped; what needs fixing is the justification it merges under.
Conventions (1)
- The description and #7448 state
-Xelide-belowis configured nowhere; sixteen modules set it toWARNING(must-fix). Rest both on the sufficient ground — no caller — and verify withgit grep -in elide, notelidable.
Simplifications (1)
- No
@elidableannotation remains after this PR, so-Xelide-below WARNINGdocuments a switch over nothing (advisory) — a follow-up, since it sits in sixteen build files.
Verification trace
Traced whether the elision was ever live, since the PR argues it was not: git grep -n Xelide -- '*.sbt' returns sixteen modules pinned at WARNING (900), and all four wrapper levels (300–800) sit below it, so every call site was compiled out. That is exactly what the deleted spec pinned — assert(counter == 0) after ElidableStatement.info { counter += 1 } is satisfiable only under elision. The deletion survives the correction intact: git grep -n ElidableStatement returns nothing at the head, so no referent is left dangling.
What changes were proposed in this PR?
Deletes
ElidableStatementand its spec. Pure deletion, no behaviour change: −175 lines.The object wraps statements in
scala.annotation.elidableso they can be compiled out below a threshold. That threshold comes from the-Xelide-belowscalac option, which appears in nobuild.sbtin this repo — so the wrapper had no effect even in principle. A case-insensitive search forelidableacross all Scala and sbt sources returns only the object and its own spec; nothing calls it.Any related issues, documentation, discussions?
Closes #7448
How was this PR tested?
Existing tests only — this PR adds none, since it removes code and the spec that covered it.
Locally, from the repo root with Java 17:
sbt "WorkflowExecutionService/Test/compile"— success.Verification, re-runnable by a reviewer:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)