string.c: keep what a string reads as across a write that cannot change it - #7180

Five in-place operations write bytes that leave the reading alone:

All five go through mrb_str_modify_keep_ascii(), which keeps a string standing at 7BIT and takes every other answer back to MRB_STR_CODERANGE_UNKNOWN. So a string holding multi-byte characters is read whole again by the next asker that needs to know whether it is sound, however little the write could have changed the answer.

This PR gives those five a prepare that keeps the answer:

static void
str_modify_keep_cr(mrb_state *mrb, struct RString *s)
{
  mrb_check_frozen(mrb, s);
  str_unshare_buffer(mrb, s);
  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_BROKEN) {
    RSTR_CODERANGE_SET(s, MRB_STR_CODERANGE_UNKNOWN);
  }
}

Only a string already read as broken has to be asked again, since a write is as likely to have mended it as to have left it broken. This is the shape CRuby's str_modify_keep_cr() has, and like CRuby's it is static: the promise it wants of a caller is that the write leaves the reading standing, nothing here can check that, and the two prepares offered outside the library are unchanged.

No answer changes

The answer a walk arrives at is the answer already on the string, so both spell the same result and only one of them pays for it. What is saved is the walk. A subject handed to a regexp is checked through mrb_str_valid_encoding_p() on every match, so a loop that edits and then matches stops re-reading the subject:

s = ("日本語 text テキスト " * 200).dup   # ~5000 bytes, read as valid UTF-8
20000.times { s.upcase!; s =~ /TEXT/; s.downcase!; s =~ /text/ }

master     0.387s
this PR    0.172s

Best of 9 runs of the bintest bin/mruby, gcc 13.3.0 -O3, x86-64.

String#index is not among the paths this helps: mrb_str_index_str() runs the check on the needle, not on the subject.

What it gives up, and where that shows

A write that leaves the string holding nothing but ASCII now keeps saying VALID where before the next walk would have settled it at 7BIT. That answer is worth less than the truth rather than being wrong, but the 7BIT || BINARY test that mrb_str_char_to_byte() and its neighbours make reads false for such a string, so its indexing walks where it could have returned the byte offset.

It does not stay that way for long. Three places walk a string and record what they found, and any of them takes the field to 7BIT:

• mrb_str_char_len(), src/string.c:670, which is String#length and every character index that needs a count, String#[] among them;
• mrb_str_valid_encoding_p(), src/string.c:705, when the string comes in UNKNOWN;
• str_ascii_only_p(), mrbgems/mruby-string-ext/src/string.c:1516, which is String#ascii_only?.

The second is the one this PR keeps away from: a string that comes in at VALID is answered off the field and never walked, so the recording never happens. That is what it costs, and where the string is long enough it costs a lot. Constructed to reach it:

s = ("a" * 400000 + "あ").dup
s.valid_encoding?   # read once, so the string comes into the write at VALID
s.chop!             # ASCII only from here: master UNKNOWN, this PR VALID
s =~ /zzz/          # master walks and records 7BIT; this PR answers off VALID
2000.times { s.rindex("a") }

master     0.0010s
this PR    0.0621s

String#rindex hands the whole search to mrb_str_byterindex_m() where the string stands at 7BIT and takes the character path otherwise, and neither path records what it walked, so nothing closes the gap from inside the loop. One String#length in front of it does:

s.length            # walks, finds nothing but ASCII, records 7BIT
2000.times { s.rindex("a") }

master     0.0010s
this PR    0.0011s

So what is given up is one walk that master would have got for free out of the regexp's check, on a string that was read as multi-byte and whose last write left it holding nothing but ASCII, and only until something counts its characters. CRuby's str_modify_keep_cr() leaves the same answer behind.

Generated code

gcc 13.3.0 -O3, x86-64, against the same objects built from master. .text of bin/mruby:

Objects differing, comparing objdump -d over every .o in the build:

It is src/string.o and nothing else in every build, and it is one inlining decision rather than five copies of a two-line body. Per symbol, bintest:

mrb_str_chomp_bang        1082 -> 2045   (+963)
mrb_str_to_s               110 ->  513   (+403)
mrb_str_downcase           186 ->  539   (+353)
mrb_str_upcase             186 ->  539   (+353)
mrb_str_capitalize_bang    201 ->  249    (+48)
mrb_str_downcase_bang      125 ->  170    (+45)
mrb_str_upcase_bang        125 ->  170    (+45)
mrb_str_chop_bang         1590 -> 1574    (-16)
mrb_string_cstr           1026 ->  205   (-821)
mrb_string_value_cstr     1076 ->  205   (-871)
str_replace                467 -> inlined away
str_unshare_buffer   inlined away ->  811

On master str_unshare_buffer() is inlined into every caller it has, including the exported mrb_str_modify_keep_ascii(), which the five bang methods then call out to. Here the five call a static instead, gcc inlines it and str_unshare_buffer() along with it, and stops inlining str_unshare_buffer() into the two cstr functions, emitting one out-of-line copy that they call. The two halves are visible in the relocations of mrb_str_chomp_bang(), master above and this branch below:

mrb_get_args   mrb_str_modify_keep_ascii   memcmp   mrb_utf8_char_head
mrb_str_valid_encoding_p   __stack_chk_fail

mrb_get_args   mrb_check_frozen   mrb_malloc x2   mrb_free x2   memcpy x2
memcmp   mrb_utf8_char_head   mrb_str_valid_encoding_p   __stack_chk_fail

and in mrb_string_value_cstr(), where master's mrb_malloc / memcpy / mrb_free are gone and a call to str_unshare_buffer stands in their place.

full-debug is the one build where gcc keeps str_modify_keep_cr() out of line, and there the whole difference is that one function at 123 bytes.

A build that indexes by byte writes no coderange, so str_modify_keep_cr() and mrb_str_modify_keep_ascii() are the same two lines there and gcc treats them the same way. byte-string and default.rb are unchanged to the byte; the one object that differs differs only in the order mrb_str_chomp_bang and mrb_str_capitalize_bang are emitted in, with every symbol the size it was.

Testing

rake -m test, all green, 0 KO, 0 crash, 0 warnings, and the same counts as master:

build_config/host-m32.rb needs a multilib toolchain, so the 32-bit build above is full-core with the compiler set to i686-linux-gnu-gcc instead.

No test comes with this. What a walk answers and what the field says agree by construction here, so there is no result for a test to tell apart; the two benchmarks above are what the change is for and what it costs.

Not in this PR

mrb_str_modify_keep_ascii() stays where it is, still exported and still called by mrb_str_modify(). Which other in-place writes could make the same promise is a separate question from giving the five that already keep it a prepare that believes them, and the coderange clearing spread across mrb_str_modify() is untouched.

Summary by CodeRabbit

• Performance
  • In-place case conversion, chomping and chopping no longer discard what a string's bytes were found to read as, so the next reader that needs it is spared a walk over the string.