feat(pyamber): resolve unset input port zero - #8264
Conversation
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8264 +/- ##
============================================
- Coverage 93.93% 93.86% -0.08%
Complexity 4762 4762
============================================
Files 1191 1191
Lines 48308 48315 +7
Branches 5377 5827 +450
============================================
- Hits 45380 45351 -29
- Misses 1494 1496 +2
- Partials 1434 1468 +34
*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:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 431 | 0.263 | 23,196/29,110/29,110 us | 🔴 +15.4% / 🔴 +86.8% |
| 🟢 | bs=100 sw=10 sl=64 | 969 | 0.591 | 102,014/124,615/124,615 us | 🟢 -28.1% / 🔴 +16.2% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,111 | 0.678 | 892,604/985,234/985,234 us | 🔴 +7.5% / 🟢 -9.1% |
Baseline details
Latest main 7d57cd6 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 431 tuples/sec | 472 tuples/sec | 787.98 tuples/sec | -8.7% | -45.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.263 MB/s | 0.288 MB/s | 0.481 MB/s | -8.7% | -45.3% |
| bs=10 sw=10 sl=64 | p50 | 23,196 us | 20,109 us | 12,593 us | +15.4% | +84.2% |
| bs=10 sw=10 sl=64 | p95 | 29,110 us | 31,885 us | 15,579 us | -8.7% | +86.8% |
| bs=10 sw=10 sl=64 | p99 | 29,110 us | 31,885 us | 18,786 us | -8.7% | +55.0% |
| bs=100 sw=10 sl=64 | throughput | 969 tuples/sec | 924 tuples/sec | 1,008 tuples/sec | +4.9% | -3.9% |
| bs=100 sw=10 sl=64 | MB/s | 0.591 MB/s | 0.564 MB/s | 0.615 MB/s | +4.8% | -3.9% |
| bs=100 sw=10 sl=64 | p50 | 102,014 us | 102,261 us | 100,701 us | -0.2% | +1.3% |
| bs=100 sw=10 sl=64 | p95 | 124,615 us | 173,429 us | 107,244 us | -28.1% | +16.2% |
| bs=100 sw=10 sl=64 | p99 | 124,615 us | 173,429 us | 116,122 us | -28.1% | +7.3% |
| bs=1000 sw=10 sl=64 | throughput | 1,111 tuples/sec | 1,131 tuples/sec | 1,042 tuples/sec | -1.8% | +6.6% |
| bs=1000 sw=10 sl=64 | MB/s | 0.678 MB/s | 0.69 MB/s | 0.636 MB/s | -1.7% | +6.6% |
| bs=1000 sw=10 sl=64 | p50 | 892,604 us | 879,093 us | 981,959 us | +1.5% | -9.1% |
| bs=1000 sw=10 sl=64 | p95 | 985,234 us | 916,533 us | 1,023,080 us | +7.5% | -3.7% |
| bs=1000 sw=10 sl=64 | p99 | 985,234 us | 916,533 us | 1,051,697 us | +7.5% | -6.3% |
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,464.36,200,128000,431,0.263,23195.63,29110.20,29110.20
1,100,10,64,20,2064.82,2000,1280000,969,0.591,102013.84,124615.43,124615.43
2,1000,10,64,20,17997.20,20000,12800000,1111,0.678,892604.44,985233.90,985233.90
Yicong-Huang
left a comment
There was a problem hiding this comment.
This is changing O(1) to O(n). if we need a better error message, you can catch the keyerror and reraise?
|
also this should be a feat instead of a fix. |
3554a43 to
45335b1
Compare
|
Kept the lookup O(1) by normalizing protobuf defaults before dictionary access. I also changed the title and commit to feat. I cannot change repository labels, so fix and release/v1.2 still need to be replaced with feature. |
What changes were proposed in this PR?
Resolve input ports by protobuf value equality instead of direct dictionary hashing. This lets an unset protobuf representation of port zero find the canonical registered port while preserving a clear error for unknown ports.
Any related issues, documentation, discussions?
Closes #8263
How was this PR tested?
The untouched live probe showed equal port-zero identities with different hashes and raised
KeyError. After the fix it reportedlookup_succeeded=Truewith the hashes still different.C:\Users\carlo\texera\texera\.venv312\Scripts\python.exe -c "import sys,pytest; sys.path[:0]=[r'C:\Users\carlo\texera\texera-worktrees\investigate-bug68\amber\src\main\python',r'C:\Users\carlo\texera\texera\amber\src\main\python']; raise SystemExit(pytest.main([r'amber/src/test/python/core/architecture/packaging/test_input_manager.py','-q','-p','no:cacheprovider']))"C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe check amber/src/main/python amber/src/test/pythonC:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe format --check amber/src/main/python/core/architecture/packaging/input_manager.py amber/src/test/python/core/architecture/packaging/test_input_manager.pyWas this PR authored or co-authored using generative AI tooling?
Generated-by: Codex