Repository navigation
fix(libdd-profiling)!: restrict ThinSlice<T> to alignment-1 types - #2742
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: c8b8399 | Docs | View more details | Give us feedback! |
966cdc4 to
462a166
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
BenchmarksComparisonBenchmark execution time: 2026-10-29 14:01:13 Comparing candidate commit c8b8399 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 13 metrics, 0 unstable metrics.
|
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| benching string interning on wordpress profile | execution_time | 115.910µs | 118.200µs ± 0.856µs | 118.114µs ± 0.202µs | 118.318µs | 119.028µs | 120.323µs | 127.078µs | 7.59% | 6.666 | 62.701 | 0.72% | 0.061µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| benching string interning on wordpress profile | execution_time | [118.082µs; 118.319µs] or [-0.100%; +0.100%] | None | None | None |
Group 2
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample_timestamped_x1000 | execution_time | 3.993ms | 4.000ms ± 0.007ms | 3.999ms ± 0.001ms | 4.000ms | 4.003ms | 4.006ms | 4.091ms | 2.30% | 11.337 | 144.869 | 0.17% | 0.000ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample_timestamped_x1000 | execution_time | [3.999ms; 4.001ms] or [-0.024%; +0.024%] | None | None | None |
Group 3
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample_frames_x1000 | execution_time | 3.995ms | 4.015ms ± 0.023ms | 4.002ms ± 0.004ms | 4.026ms | 4.056ms | 4.096ms | 4.150ms | 3.70% | 2.241 | 7.189 | 0.57% | 0.002ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample_frames_x1000 | execution_time | [4.012ms; 4.018ms] or [-0.079%; +0.079%] | None | None | None |
Group 4
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_serialize_compressed_pprof_timestamped_x1000 | execution_time | 850.362µs | 853.202µs ± 2.047µs | 852.600µs ± 1.019µs | 854.058µs | 856.767µs | 860.843µs | 863.550µs | 1.28% | 1.688 | 4.185 | 0.24% | 0.145µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_serialize_compressed_pprof_timestamped_x1000 | execution_time | [852.919µs; 853.486µs] or [-0.033%; +0.033%] | None | None | None |
Group 5
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample2_frames_x1000 | execution_time | 704.690µs | 706.611µs ± 0.684µs | 706.626µs ± 0.543µs | 707.068µs | 707.735µs | 708.071µs | 708.256µs | 0.23% | 0.003 | -0.348 | 0.10% | 0.048µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample2_frames_x1000 | execution_time | [706.516µs; 706.706µs] or [-0.013%; +0.013%] | None | None | None |
Group 6
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | c8b8399 | 1793281887 | yannham/fix-unsoundness-4 |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profiles_dictionary/profile_string_inserts/threads/1 | execution_time | 314.484µs | 318.344µs ± 2.345µs | 319.430µs ± 1.561µs | 319.682µs | 321.001µs | 321.072µs | 321.090µs | 0.52% | -0.452 | -1.262 | 0.70% | 0.742µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/1 | throughput | 3189138.557op/s | 3216800.132op/s ± 23770.168op/s | 3205710.722op/s ± 15590.620op/s | 3237719.728op/s | 3251129.019op/s | 3255128.648op/s | 3256128.556op/s | 1.57% | 0.463 | -1.254 | 0.70% | 7516.787op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/16 | execution_time | 1.905ms | 1.913ms ± 0.005ms | 1.913ms ± 0.003ms | 1.914ms | 1.921ms | 1.923ms | 1.923ms | 0.54% | 0.537 | -0.179 | 0.26% | 0.002ms | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/16 | throughput | 8519814.474op/s | 8565448.847op/s ± 23135.705op/s | 8565767.521op/s ± 13615.592op/s | 8580564.129op/s | 8593328.354op/s | 8599264.825op/s | 8600748.943op/s | 0.41% | -0.525 | -0.189 | 0.26% | 7316.152op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/2 | execution_time | 556.942µs | 560.017µs ± 1.693µs | 559.968µs ± 1.151µs | 561.083µs | 562.347µs | 562.841µs | 562.965µs | 0.54% | -0.025 | -0.291 | 0.29% | 0.535µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/2 | throughput | 3637882.291op/s | 3657059.634op/s ± 11055.074op/s | 3657352.426op/s ± 7518.894op/s | 3662310.786op/s | 3672480.678op/s | 3676273.824op/s | 3677222.111op/s | 0.54% | 0.040 | -0.287 | 0.29% | 3495.921op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/4 | execution_time | 740.873µs | 744.715µs ± 3.549µs | 743.644µs ± 1.094µs | 745.771µs | 750.637µs | 752.716µs | 753.236µs | 1.29% | 1.462 | 1.367 | 0.45% | 1.122µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/4 | throughput | 5437872.684op/s | 5500199.376op/s ± 26040.390op/s | 5508014.033op/s ± 8111.567op/s | 5515389.594op/s | 5523045.367op/s | 5527500.594op/s | 5528614.400op/s | 0.37% | -1.445 | 1.324 | 0.45% | 8234.695op/s | 1 | 10 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profiles_dictionary/profile_string_inserts/threads/1 | execution_time | [316.891µs; 319.798µs] or [-0.457%; +0.457%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/1 | throughput | [3202067.500op/s; 3231532.765op/s] or [-0.458%; +0.458%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/16 | execution_time | [1.910ms; 1.916ms] or [-0.168%; +0.168%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/16 | throughput | [8551109.452op/s; 8579788.242op/s] or [-0.167%; +0.167%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/2 | execution_time | [558.968µs; 561.067µs] or [-0.187%; +0.187%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/2 | throughput | [3650207.754op/s; 3663911.514op/s] or [-0.187%; +0.187%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/4 | execution_time | [742.516µs; 746.915µs] or [-0.295%; +0.295%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/4 | throughput | [5484059.672op/s; 5516339.081op/s] or [-0.293%; +0.293%] | None | None | None |
Baseline
Baseline benchmark details
Group 1
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| benching string interning on wordpress profile | execution_time | 115.870µs | 117.503µs ± 0.644µs | 117.464µs ± 0.272µs | 117.773µs | 118.168µs | 118.745µs | 123.606µs | 5.23% | 4.175 | 39.476 | 0.55% | 0.046µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| benching string interning on wordpress profile | execution_time | [117.414µs; 117.592µs] or [-0.076%; +0.076%] | None | None | None |
Group 2
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample_timestamped_x1000 | execution_time | 3.989ms | 3.996ms ± 0.007ms | 3.995ms ± 0.001ms | 3.997ms | 3.999ms | 4.001ms | 4.083ms | 2.21% | 11.577 | 149.070 | 0.17% | 0.000ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample_timestamped_x1000 | execution_time | [3.995ms; 3.997ms] or [-0.023%; +0.023%] | None | None | None |
Group 3
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample_frames_x1000 | execution_time | 4.010ms | 4.017ms ± 0.003ms | 4.016ms ± 0.002ms | 4.018ms | 4.021ms | 4.025ms | 4.025ms | 0.24% | 0.630 | 0.408 | 0.07% | 0.000ms | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample_frames_x1000 | execution_time | [4.016ms; 4.017ms] or [-0.010%; +0.010%] | None | None | None |
Group 4
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_serialize_compressed_pprof_timestamped_x1000 | execution_time | 861.432µs | 865.351µs ± 2.421µs | 865.035µs ± 1.616µs | 866.865µs | 869.758µs | 871.709µs | 874.563µs | 1.10% | 0.841 | 0.651 | 0.28% | 0.171µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_serialize_compressed_pprof_timestamped_x1000 | execution_time | [865.016µs; 865.687µs] or [-0.039%; +0.039%] | None | None | None |
Group 5
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profile_add_sample2_frames_x1000 | execution_time | 708.925µs | 710.719µs ± 0.728µs | 710.681µs ± 0.497µs | 711.193µs | 712.075µs | 712.390µs | 712.904µs | 0.31% | 0.434 | -0.129 | 0.10% | 0.051µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profile_add_sample2_frames_x1000 | execution_time | [710.618µs; 710.820µs] or [-0.014%; +0.014%] | None | None | None |
Group 6
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | 9ca1e4f | 1793281602 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| profiles_dictionary/profile_string_inserts/threads/1 | execution_time | 313.658µs | 316.270µs ± 1.885µs | 316.479µs ± 1.452µs | 317.368µs | 318.854µs | 319.243µs | 319.340µs | 0.90% | -0.008 | -1.003 | 0.57% | 0.596µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/1 | throughput | 3206609.702op/s | 3237843.326op/s ± 19301.402op/s | 3235604.240op/s ± 14777.960op/s | 3253082.407op/s | 3263987.784op/s | 3264558.730op/s | 3264701.467op/s | 0.90% | 0.025 | -1.012 | 0.57% | 6103.639op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/16 | execution_time | 1.905ms | 1.921ms ± 0.009ms | 1.920ms ± 0.007ms | 1.928ms | 1.932ms | 1.933ms | 1.933ms | 0.67% | -0.288 | -0.850 | 0.44% | 0.003ms | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/16 | throughput | 8476537.758op/s | 8530271.491op/s ± 39230.899op/s | 8533075.918op/s ± 32637.653op/s | 8551262.081op/s | 8587168.885op/s | 8598646.417op/s | 8601515.800op/s | 0.80% | 0.302 | -0.834 | 0.44% | 12405.899op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/2 | execution_time | 557.340µs | 560.057µs ± 1.686µs | 559.949µs ± 1.363µs | 561.398µs | 562.041µs | 562.180µs | 562.215µs | 0.40% | -0.433 | -0.927 | 0.29% | 0.533µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/2 | throughput | 3642733.189op/s | 3656798.096op/s ± 11024.915op/s | 3657474.164op/s ± 8878.029op/s | 3660541.701op/s | 3674177.396op/s | 3674510.866op/s | 3674594.234op/s | 0.47% | 0.441 | -0.922 | 0.29% | 3486.384op/s | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/4 | execution_time | 734.979µs | 736.999µs ± 1.675µs | 736.517µs ± 0.857µs | 737.605µs | 739.817µs | 739.820µs | 739.821µs | 0.45% | 0.794 | -0.648 | 0.22% | 0.530µs | 1 | 10 |
| profiles_dictionary/profile_string_inserts/threads/4 | throughput | 5536473.578op/s | 5557699.398op/s ± 12612.739op/s | 5561307.652op/s ± 6474.927op/s | 5566431.646op/s | 5570630.154op/s | 5572484.994op/s | 5572948.704op/s | 0.21% | -0.790 | -0.652 | 0.22% | 3988.498op/s | 1 | 10 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| profiles_dictionary/profile_string_inserts/threads/1 | execution_time | [315.102µs; 317.438µs] or [-0.369%; +0.369%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/1 | throughput | [3225880.413op/s; 3249806.239op/s] or [-0.369%; +0.369%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/16 | execution_time | [1.915ms; 1.926ms] or [-0.285%; +0.285%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/16 | throughput | [8505956.375op/s; 8554586.607op/s] or [-0.285%; +0.285%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/2 | execution_time | [559.012µs; 561.103µs] or [-0.187%; +0.187%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/2 | throughput | [3649964.908op/s; 3663631.284op/s] or [-0.187%; +0.187%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/4 | execution_time | [735.961µs; 738.037µs] or [-0.141%; +0.141%] | None | None | None |
| profiles_dictionary/profile_string_inserts/threads/4 | throughput | [5549882.084op/s; 5565516.711op/s] or [-0.141%; +0.141%] | None | None | None |
ThinSlice<T> laid out its backing allocation with alignment 1 (8-byte length prefix immediately followed by the elements), but created &[T] and &InlineSlice<T> references into it. For any T with alignment > 1 those references are misaligned, which is undefined behavior reachable from safe code through the public generic SliceSet<T> / ParallelSliceSet<T> (e.g. try_insert with T = u128, reproduced with Miri: "accessing memory with alignment 4, but alignment 16 is required"). Computing a real per-type layout would require padding the size prefix per alignment and reworking the ThinPtr/InlineSlice/ConstString format shared with ThinStr, for a type where only u8 is used today. Instead, enforce the alignment-1 requirement at compile time with a const assert in layout_for, the single choke point every safe construction and deallocation path goes through. Instantiating ThinSlice<T> (and hence SliceSet<T>/ParallelSliceSet<T>) with an aligned T is now a compile error instead of latent UB.
462a166 to
c8b8399
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
5cc4410
into
main
What does this PR do?
This PR restricts
ThinSlice<T>to 1-aligned types with a compile-time check. InstantiatingThinSlice<T>(and henceSliceSet<T>/ParallelSliceSet<T>) with aTthat has a higher alignement is now a compile error instead.Motivation
ThinSlice<T>laid out its backing allocation with alignment 1 (an 8-byte length prefix immediately followed by the elements), but created&[T]and&InlineSlice<T>references into it. For anyTwith alignment > 1 those references are misaligned, ie UB accessible from safe code. Confirmed with a small reproducer on Miri.No UB was triggered so far, because all callsites would use
T=u8.Additional Notes
Another possibility would be to add padding depending on the alignment of
T, but this would require more changes to a bunch of things in the implementation of several related types. Since we're only using it foru8, a compile time check is much simpler to prevent future UB for now.How to test the change?
cargo build -p libdd-profilingwith a temporaryThinSlice<u128>use fails witherror[E0080]: ThinSlice<T> requires T to have alignment 1...(verified on MSRV 1.91.1 and nightly).ThinSlice::try_allocate_for::<u128>+try_from_slice_in+as_slicetriggersaccessing memory with alignment 4, but alignment 16 is required.