Issue #22186 has been updated by byroot (Jean Boussier).
The slot size seems to matter more than the memcpy itself.
Right, that is often the case in micro-benchmarks, larger pools have proportionately less slots, hence end up triggering GC more often tanking performance. This isn't always a good reflection of performance in larger benchmark / real programs. Aside from these comments on the potential regression, ultimately, just like with shareable middle substring, I think such a change will speedup some programs, and slow down others. Sometimes the slice into another string will be long lived or immediately mutated and copying is preferable. But sometimes it is very short lived and sharing would have been preferable. 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. I would be in favor of letting the caller chose, as I'm frequently working with performance sensitive string parsing code, buffers, etc, but I wouldn't expect such design to be accepted. ---------------------------------------- Feature #22186: Increase the embeddable size limit for substrings created by `str_subseq()` https://bugs.ruby-lang.org/issues/22186#change-117959 * 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/