Issue #22186 has been updated by byroot (Jean Boussier). My worry about this is that there might be a cutoff point where copying the bytes in a new embedded string is more costly than going the shared route. There's also the memory usage concern. Only copying when we fit in the smallest slot is definitely not good, but I think here a micro-benchmark with various sizes (50, 100, 250, 500, 1000) would be warranted. ---------------------------------------- Feature #22186: Increase the embeddable size limit for substrings created by `str_subseq()` https://bugs.ruby-lang.org/issues/22186#change-117948 * 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) -- https://bugs.ruby-lang.org/