Issue #22186 has been updated by himura467 (Akito Shitara). File output_004.box-ratio.png added File output_004.txt added byroot (Jean Boussier) wrote in #note-13:
We could also have a different limit depending on the status of the source string. If the source string is already frozen or shared, creating an extra shared string is much cheaper, so we could keep the current embeddable size in these case, but otherwise it would be ~128B (or close, it doesn't have to perfectly fit an existing slot size, but might as well).
Updated the implementation following this. The cutoff is a slot of 128 bytes, and a frozen or already shared source keeps the default `struct RString` slot as before. Benchmarks below. ---------------------------------------- Feature #22186: Increase the embeddable size limit for substrings created by `str_subseq()` https://bugs.ruby-lang.org/issues/22186#change-118376 * 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) output_004.box-ratio.png (239 KB) output_004.txt (8.81 KB) -- https://bugs.ruby-lang.org/