Issue #22186 has been updated by himura467 (Akito Shitara). byroot (Jean Boussier) wrote in #note-6:
Since the caller doesn't have control over whether Ruby will use copy or sharing, ultimately we're limited to heuristics, which by definition won't ever be perfect.
Agreed, and I should have said so alongside the numbers. I did not mean the header data as an argument that either cutoff is the correct one, since there isn't one. It is only a lens for choosing, and the choice stays a judgement call. ---------------------------------------- Feature #22186: Increase the embeddable size limit for substrings created by `str_subseq()` https://bugs.ruby-lang.org/issues/22186#change-118139 * Author: himura467 (Akito Shitara) * Status: Open ---------------------------------------- ## Summary `str_subseq()` unnecessarily limits how large a sharable middle substring can be while still being embedded into the new `RString`. Substrings roughly 24 to 1000 bytes long (on a 64-bit build) always take the shared path, even though variable width allocation (VWA, Feature #18239) already supports embedding objects of that size. I propose sizing the allocation to the actual substring length via `STR_EMBEDDABLE_P()` / `str_alloc_embed()`, the same pattern used by other allocation sites in `string.c`, instead of allocating through `str_alloc_heap()` and checking whether the substring fits in the smallest heap slot class. ## Background For a sharable substring (per `SHARABLE_SUBSTRING_P`), `str_subseq()` either embeds the bytes directly into the new `RString`, or shares the parent's heap buffer, which keeps the parent alive. Since [commit 132f097149](https://github.com/ruby/ruby/commit/132f097149af36cb77308d9fe1c1a94940ab2089), the decision has been: ```c str2 = str_alloc_heap(rb_cString); if (str_embed_capa(str2) >= len + termlen) { // embed } else { // share } ``` `str_alloc_heap()` always allocates the default `sizeof(struct RString)` slot regardless of `len`, so `str_embed_capa(str2)` is capped at roughly 23 bytes. Meanwhile the default GC's size pools go up to 1024 bytes on 64-bit builds, and other allocation sites (`str_enc_new` behind `rb_str_new`, `rb_str_buf_new`, `str_new_frozen_buffer`, `rb_str_times`, ...) already pick the right-sized slot via `STR_EMBEDDABLE_P()` / `str_alloc_embed()`. ## Proposed change Draft PR: https://github.com/ruby/ruby/pull/17723 Check `STR_EMBEDDABLE_P(len, termlen)` up front and allocate with `str_alloc_embed()`; only allocate the `STR_NOEMBED` heap object on the sharing branch. Behavior is unchanged for substrings that aren't sharable or that reach the end of the parent's buffer. Only the threshold at which a sharable middle substring switches from embedding to sharing grows. ## Benchmark results Using [ruby-bench](https://github.com/ruby/ruby-bench), this branch vs current master (`arm64-darwin25`, `+PRISM`). Full results are attached as `output_001.txt`, with a per-benchmark ratio chart in `output_001-ratio-current.png`. Top 3 and bottom 3 by ratio (master/branch, higher is better): | bench | master (ms) | current (ms) | ratio | | ---- | ---- | ---- | ----- | | fluentd | 258.6 | 230.8 | 1.120 | | ruby-json | 154.1 | 150.2 | 1.026 | | liquid-c | 31.2 | 30.6 | 1.022 | | graphql-native | 172.0 | 174.9 | 0.983 | | activerecord | 148.7 | 153.0 | 0.972 | | psych-load | 1244.9 | 1291.4 | 0.964 | Note that most except fluentd varies between runs. The fluentd improvement reproduces across runs, while the entries in the bottom 3 do not. So the few 2-4% slowdowns may be noise, but I'd like to verify them with more runs and machines before treating this as risk-free. ## Relation to Feature #19315 #19315 (sharable middle substrings) addresses the same underlying problem: middle substrings copy unnecessarily. The branch benchmarked in https://bugs.ruby-lang.org/issues/19315#note-52 actually included this embed size change as one of its commits, in addition to enabling `SHARABLE_MIDDLE_SUBSTRING`. To isolate the two, I benchmarked a variant of that branch with only the embed size change reverted ([rstring-raw-ptr-reverted](https://github.com/himura467/ruby/tree/rstring-raw-ptr-reverted)), alongside this proposal, against the same master. Full results are attached as `output_002.txt`: | bench | master (ms) | this proposal (ms) | #19315 without this change (ms) | | ---- | ---- | ---- | ---- | | fluentd | 256.9 | 207.2 (x1.240) | 255.6 (x1.005) | The clear `fluentd` improvement seen in note-52 reproduces with this change alone, and disappears when this change is reverted from the #19315 branch. So the practical win observed on ruby-bench so far is attributable to the larger embed threshold rather than to `SHARABLE_MIDDLE_SUBSTRING` itself. Beyond that, this change is much narrower: - A self-contained fix to one function, reusing the VWA embedding infrastructure already battle-tested elsewhere in `string.c`. - No API/ABI implications; `RSTRING_PTR()`'s `\0`-termination guarantee is untouched. - No memory-retention risk, since embedding copies the bytes instead of referencing the parent. The two are orthogonal (#19315 still helps for slices too large to embed, as its micro-benchmarks show), but this smaller fix seems worth landing first on its own merits. ---Files-------------------------------- output_001.txt (7.76 KB) output_001-ratio-current.png (184 KB) output_002.txt (14.9 KB) string_subseq.yml (418 Bytes) str_subseq_memory.rb (1.25 KB) -- https://bugs.ruby-lang.org/