ruby-changes:45868
From: naruse <ko1@a...>
Date: Mon, 13 Mar 2017 03:15:37 +0900 (JST)
Subject: [ruby-changes:45868] naruse:r57941 (ruby_2_4): merge revision(s) 57469, 57471, 57472, 57503, 57508: [Backport #13299]
naruse 2017-03-13 03:15:33 +0900 (Mon, 13 Mar 2017) New Revision: 57941 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=57941 Log: merge revision(s) 57469,57471,57472,57503,57508: [Backport #13299] io.c: recycle garbage on write * string.c (STR_IS_SHARED_M): new flag to mark shared mulitple times (STR_SET_SHARED): set STR_IS_SHARED_M (rb_str_tmp_frozen_acquire, rb_str_tmp_frozen_release): new functions (str_new_frozen): set/unset STR_IS_SHARED_M as appropriate * internal.h: declare new functions * io.c (fwrite_arg, fwrite_do, fwrite_end): new (io_fwrite): use new functions Introduce rb_str_tmp_frozen_acquire and rb_str_tmp_frozen_release to manage a hidden, frozen string. Reuse one bit of the embed length for shared strings as STR_IS_SHARED_M to indicate a string has been shared multiple times. In the common case, the string is only shared once so the object slot can be reclaimed immediately. minimum results in each 3 measurements. (time and size) Execution time (sec) name trunk built io_copy_stream_write 0.682 0.254 io_copy_stream_write_socket 1.225 0.751 Speedup ratio: compare with the result of `trunk' (greater is better) name built io_copy_stream_write 2.680 io_copy_stream_write_socket 1.630 Memory usage (last size) (B) name trunk built io_copy_stream_write 95436800.000 6512640.000 io_copy_stream_write_socket 117628928.000 7127040.000 Memory consuming ratio (size) with the result of `trunk' (greater is better) name built io_copy_stream_write 14.654 io_copy_stream_write_socket 16.505 string.c (rb_str_tmp_frozen_release): release embedded strings Handle the embedded case first, since we may have an embedded duplicate and non-embedded original string. * string.c (rb_str_tmp_frozen_release): handled embedded strings * test/ruby/test_io.rb (test_write_no_garbage): new test [ruby-core:78898] [Bug #13085] io.c (rb_io_syswrite): avoid leaving garbage after write As with IO#write, IO#syswrite also generates garbage which can be harmful in hand-coded read-write loops. * io.c (swrite_arg, swrite_do, swrite_end): new (rb_io_syswrite): use new functions to cleanup garbage [ruby-core:78898] [Bug #13085] Add class name to assert messages io.c: remove rb_ensure usage for rb_str_tmp_frozen_* calls Using rb_ensure pessimizes the common case and makes the code more difficult to read and follow. If we hit an exceptions during write, just let the GC handle cleanup as the exception is already bad for garbage. * io.c (io_fwrite): call rb_str_tmp_frozen{acquire,release} directly (rb_io_syswrite): ditto (fwrite_do, fwrite_end, swrite_do, swrite_end): remove Modified directories: branches/ruby_2_4/ Modified files: branches/ruby_2_4/internal.h branches/ruby_2_4/io.c branches/ruby_2_4/string.c branches/ruby_2_4/test/ruby/test_io.rb branches/ruby_2_4/version.h Index: ruby_2_4/internal.h =================================================================== --- ruby_2_4/internal.h (revision 57940) +++ ruby_2_4/internal.h (revision 57941) @@ -1459,6 +1459,8 @@ VALUE rb_id_quote_unprintable(ID); https://github.com/ruby/ruby/blob/trunk/ruby_2_4/internal.h#L1459 char *rb_str_fill_terminator(VALUE str, const int termlen); void rb_str_change_terminator_length(VALUE str, const int oldtermlen, const int termlen); VALUE rb_str_locktmp_ensure(VALUE str, VALUE (*func)(VALUE), VALUE arg); +VALUE rb_str_tmp_frozen_acquire(VALUE str); +void rb_str_tmp_frozen_release(VALUE str, VALUE tmp); VALUE rb_str_chomp_string(VALUE str, VALUE chomp); #ifdef RUBY_ENCODING_H VALUE rb_external_str_with_enc(VALUE str, rb_encoding *eenc); Index: ruby_2_4/test/ruby/test_io.rb =================================================================== --- ruby_2_4/test/ruby/test_io.rb (revision 57940) +++ ruby_2_4/test/ruby/test_io.rb (revision 57941) @@ -3508,5 +3508,23 @@ __END__ https://github.com/ruby/ruby/blob/trunk/ruby_2_4/test/ruby/test_io.rb#L3508 end end; end + + def test_write_no_garbage + res = {} + ObjectSpace.count_objects(res) # creates strings on first call + [ 'foo'.b, '*' * 24 ].each do |buf| + with_pipe do |r, w| + before = ObjectSpace.count_objects(res)[:T_STRING] + n = w.write(buf) + s = w.syswrite(buf) + after = ObjectSpace.count_objects(res)[:T_STRING] + assert_equal before, after, + 'no strings left over after write [ruby-core:78898] [Bug #13085]' + assert_not_predicate buf, :frozen?, 'no inadvertant freeze' + assert_equal buf.bytesize, n, 'IO#write wrote expected size' + assert_equal s, n, 'IO#syswrite wrote expected size' + end + end + end end end Index: ruby_2_4/version.h =================================================================== --- ruby_2_4/version.h (revision 57940) +++ ruby_2_4/version.h (revision 57941) @@ -1,6 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_4/version.h#L1 #define RUBY_VERSION "2.4.0" #define RUBY_RELEASE_DATE "2017-03-13" -#define RUBY_PATCHLEVEL 95 +#define RUBY_PATCHLEVEL 96 #define RUBY_RELEASE_YEAR 2017 #define RUBY_RELEASE_MONTH 3 Index: ruby_2_4/string.c =================================================================== --- ruby_2_4/string.c (revision 57940) +++ ruby_2_4/string.c (revision 57941) @@ -70,6 +70,7 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L70 * 1: RSTRING_NOEMBED * 2: STR_SHARED (== ELTS_SHARED) * 2-6: RSTRING_EMBED_LEN (5 bits == 32) + * 6: STR_IS_SHARED_M (shared, when RSTRING_NOEMBED==1 && klass==0) * 7: STR_TMPLOCK * 8-9: ENC_CODERANGE (2 bits) * 10-16: ENCODING (7 bits == 128) @@ -79,6 +80,7 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L80 */ #define RUBY_MAX_CHAR_LEN 16 +#define STR_IS_SHARED_M FL_USER6 #define STR_TMPLOCK FL_USER7 #define STR_NOFREE FL_USER18 #define STR_FAKESTR FL_USER19 @@ -150,6 +152,8 @@ VALUE rb_cSymbol; https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L152 if (!FL_TEST(str, STR_FAKESTR)) { \ RB_OBJ_WRITE((str), &RSTRING(str)->as.heap.aux.shared, (shared_str)); \ FL_SET((str), STR_SHARED); \ + if (RBASIC_CLASS((shared_str)) == 0) /* for CoW-friendliness */ \ + FL_SET_RAW((shared_str), STR_IS_SHARED_M); \ } \ } while (0) @@ -1127,6 +1131,45 @@ rb_str_new_frozen(VALUE orig) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L1131 return str; } +VALUE +rb_str_tmp_frozen_acquire(VALUE orig) +{ + VALUE tmp; + + if (OBJ_FROZEN_RAW(orig)) return orig; + + tmp = str_new_frozen(0, orig); + OBJ_INFECT(tmp, orig); + + return tmp; +} + +void +rb_str_tmp_frozen_release(VALUE orig, VALUE tmp) +{ + if (RBASIC_CLASS(tmp) != 0) + return; + + if (STR_EMBED_P(tmp)) { + assert(OBJ_FROZEN_RAW(tmp)); + rb_gc_force_recycle(tmp); + } + else if (FL_TEST_RAW(orig, STR_SHARED) && + !FL_TEST_RAW(orig, STR_TMPLOCK|RUBY_FL_FREEZE)) { + VALUE shared = RSTRING(orig)->as.heap.aux.shared; + + if (shared == tmp && !FL_TEST_RAW(tmp, STR_IS_SHARED_M)) { + FL_UNSET_RAW(orig, STR_SHARED); + assert(RSTRING(orig)->as.heap.ptr == RSTRING(tmp)->as.heap.ptr); + assert(RSTRING(orig)->as.heap.len == RSTRING(tmp)->as.heap.len); + RSTRING(orig)->as.heap.aux.capa = RSTRING(tmp)->as.heap.aux.capa; + RBASIC(orig)->flags |= RBASIC(tmp)->flags & STR_NOFREE; + assert(OBJ_FROZEN_RAW(tmp)); + rb_gc_force_recycle(tmp); + } + } +} + static VALUE str_new_frozen(VALUE klass, VALUE orig) { @@ -1152,6 +1195,8 @@ str_new_frozen(VALUE klass, VALUE orig) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L1195 RSTRING(str)->as.heap.len -= ofs + rest; } else { + if (RBASIC_CLASS(shared) == 0) + FL_SET_RAW(shared, STR_IS_SHARED_M); return shared; } } @@ -1171,6 +1216,8 @@ str_new_frozen(VALUE klass, VALUE orig) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/string.c#L1216 RBASIC(str)->flags |= RBASIC(orig)->flags & STR_NOFREE; RBASIC(orig)->flags &= ~STR_NOFREE; STR_SET_SHARED(orig, str); + if (klass == 0) + FL_UNSET_RAW(str, STR_IS_SHARED_M); } } Index: ruby_2_4/io.c =================================================================== --- ruby_2_4/io.c (revision 57940) +++ ruby_2_4/io.c (revision 57941) @@ -1423,6 +1423,9 @@ static long https://github.com/ruby/ruby/blob/trunk/ruby_2_4/io.c#L1423 io_fwrite(VALUE str, rb_io_t *fptr, int nosync) { int converted = 0; + VALUE tmp; + long n, len; + const char *ptr; #ifdef _WIN32 if (fptr->mode & FMODE_TTY) { long len = rb_w32_write_console(str, fptr->fd); @@ -1432,11 +1435,13 @@ io_fwrite(VALUE str, rb_io_t *fptr, int https://github.com/ruby/ruby/blob/trunk/ruby_2_4/io.c#L1435 str = do_writeconv(str, fptr, &converted); if (converted) OBJ_FREEZE(str); - else - str = rb_str_new_frozen(str); - return io_binwrite(str, RSTRING_PTR(str), RSTRING_LEN(str), - fptr, nosync); + tmp = rb_str_tmp_frozen_acquire(str); + RSTRING_GETMEM(tmp, ptr, len); + n = io_binwrite(tmp, ptr, len, fptr, nosync); + rb_str_tmp_frozen_release(str, tmp); + + return n; } ssize_t @@ -4727,8 +4732,10 @@ rb_io_sysseek(int argc, VALUE *argv, VAL https://github.com/ruby/ruby/blob/trunk/ruby_2_4/io.c#L4732 static VALUE rb_io_syswrite(VALUE io, VALUE str) { + VALUE tmp; rb_io_t *fptr; - long n; + long n, len; + const char *ptr; if (!RB_TYPE_P(str, T_STRING)) str = rb_obj_as_string(str); @@ -4737,16 +4744,15 @@ rb_io_syswrite(VALUE io, VALUE str) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/io.c#L4744 GetOpenFile(io, fptr); rb_io_check_writable(fptr); - str = rb_str_new_frozen(str); - if (fptr->wbuf.len) { rb_warn("syswrite for buffered IO"); } - n = rb_write_internal(fptr->fd, RSTRING_PTR(str), RSTRING_LEN(str)); - RB_GC_GUARD(str); - + tmp = rb_str_tmp_frozen_acquire(str); + RSTRING_GETMEM(tmp, ptr, len); + n = rb_write_internal(fptr->fd, ptr, len); if (n == -1) rb_sys_fail_path(fptr->pathv); + rb_str_tmp_frozen_release(str, tmp); return LONG2FIX(n); } Property changes on: ruby_2_4 ___________________________________________________________________ Modified: svn:mergeinfo Merged /trunk:r57469,57471-57472,57503,57508 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/