ruby-changes:62973
From: Jean <ko1@a...>
Date: Wed, 16 Sep 2020 01:18:31 +0900 (JST)
Subject: [ruby-changes:62973] fbba6bd4e3 (master): Parse ObjectSpace.dump_all / dump arguments in Ruby to avoid allocation noise
https://git.ruby-lang.org/ruby.git/commit/?id=fbba6bd4e3 From fbba6bd4e3dff7a61965208fecae908f10c4edbe Mon Sep 17 00:00:00 2001 From: Jean Boussier <jean.boussier@g...> Date: Thu, 10 Sep 2020 13:17:53 +0200 Subject: Parse ObjectSpace.dump_all / dump arguments in Ruby to avoid allocation noise [Feature #17045] ObjectSpace.dump_all should allocate as little as possible in the GC heap Up until this commit ObjectSpace.dump_all allocates two Hash because of `rb_scan_args`. It also can allocate a `File` because of `rb_io_get_write_io`. These allocations are problematic because `dump_all` dumps the Ruby heap, so it should try modify as little as possible what it is observing. diff --git a/ext/objspace/lib/objspace.rb b/ext/objspace/lib/objspace.rb new file mode 100644 index 0000000..7cd7507 --- /dev/null +++ b/ext/objspace/lib/objspace.rb @@ -0,0 +1,87 @@ https://github.com/ruby/ruby/blob/trunk/ext/objspace/lib/objspace.rb#L1 +# frozen_string_literal: true + +require 'objspace.so' + +module ObjectSpace + class << self + private :_dump + private :_dump_all + end + + module_function + + # call-seq: + # ObjectSpace.dump(obj[, output: :string]) # => "{ ... }" + # ObjectSpace.dump(obj, output: :file) # => #<File:/tmp/rubyobj20131125-88733-1xkfmpv.json> + # ObjectSpace.dump(obj, output: :stdout) # => nil + # + # Dump the contents of a ruby object as JSON. + # + # This method is only expected to work with C Ruby. + # This is an experimental method and is subject to change. + # In particular, the function signature and output format are + # not guaranteed to be compatible in future versions of ruby. + def dump(obj, output: :string) + out = case output + when :file, nil + require 'tempfile' + Tempfile.create(%w(rubyobj .json)) + when :stdout + STDOUT + when :string + +'' + when IO + output + else + raise ArgumentError, "wrong output option: #{output.inspect}" + end + + _dump(obj, out) + end + + + # call-seq: + # ObjectSpace.dump_all([output: :file]) # => #<File:/tmp/rubyheap20131125-88469-laoj3v.json> + # ObjectSpace.dump_all(output: :stdout) # => nil + # ObjectSpace.dump_all(output: :string) # => "{...}\n{...}\n..." + # ObjectSpace.dump_all(output: + # File.open('heap.json','w')) # => #<File:heap.json> + # ObjectSpace.dump_all(output: :string, + # since: 42) # => "{...}\n{...}\n..." + # + # Dump the contents of the ruby heap as JSON. + # + # _since_ must be a non-negative integer or +nil+. + # + # If _since_ is a positive integer, only objects of that generation and + # newer generations are dumped. The current generation can be accessed using + # GC::count. + # + # Objects that were allocated without object allocation tracing enabled + # are ignored. See ::trace_object_allocations for more information and + # examples. + # + # If _since_ is omitted or is +nil+, all objects are dumped. + # + # This method is only expected to work with C Ruby. + # This is an experimental method and is subject to change. + # In particular, the function signature and output format are + # not guaranteed to be compatible in future versions of ruby. + def dump_all(output: :file, full: false, since: nil) + out = case output + when :file, nil + require 'tempfile' + Tempfile.create(%w(rubyheap .json)) + when :stdout + STDOUT + when :string + +'' + when IO + output + else + raise ArgumentError, "wrong output option: #{output.inspect}" + end + + _dump_all(out, full, since) + end +end diff --git a/ext/objspace/objspace_dump.c b/ext/objspace/objspace_dump.c index f81855c..e12b553 100644 --- a/ext/objspace/objspace_dump.c +++ b/ext/objspace/objspace_dump.c @@ -25,9 +25,6 @@ https://github.com/ruby/ruby/blob/trunk/ext/objspace/objspace_dump.c#L25 RUBY_EXTERN const char ruby_hexdigits[]; -static VALUE sym_output, sym_stdout, sym_string, sym_file; -static VALUE sym_full, sym_since; - #define BUFFER_CAPACITY 4096 struct dump_config { @@ -539,142 +536,62 @@ root_obj_i(const char *category, VALUE obj, void *data) https://github.com/ruby/ruby/blob/trunk/ext/objspace/objspace_dump.c#L536 dc->roots = 1; } -static VALUE -dump_output(struct dump_config *dc, VALUE opts, VALUE output, const char *filename) +static void +dump_output(struct dump_config *dc, VALUE output, VALUE full, VALUE since) { - VALUE tmp; dc->full_heap = 0; dc->buffer_len = 0; - if (RTEST(opts)) { - output = rb_hash_aref(opts, sym_output); - - if (Qtrue == rb_hash_lookup2(opts, sym_full, Qfalse)) - dc->full_heap = 1; - - VALUE since = rb_hash_aref(opts, sym_since); - if (RTEST(since)) { - dc->partial_dump = 1; - dc->since = NUM2SIZET(since); - } else { - dc->partial_dump = 0; - } + if (TYPE(output) == T_STRING) { + dc->stream = Qfalse; + dc->string = output; + } else { + dc->stream = output; + dc->string = Qfalse; } - if (output == sym_stdout) { - dc->stream = rb_stdout; - dc->string = Qnil; - } - else if (output == sym_file || output == Qnil) { - rb_require("tempfile"); - tmp = rb_assoc_new(rb_str_new_cstr(filename), rb_str_new_cstr(".json")); - tmp = rb_funcallv(rb_path2class("Tempfile"), rb_intern("create"), 1, &tmp); - io: - dc->string = Qnil; - dc->stream = rb_io_get_write_io(tmp); - } - else if (output == sym_string) { - dc->string = rb_str_new_cstr(""); - } - else if (!NIL_P(tmp = rb_io_check_io(output))) { - output = sym_file; - goto io; - } - else { - rb_raise(rb_eArgError, "wrong output option: %"PRIsVALUE, output); + if (full == Qtrue) { + dc->full_heap = 1; } - return output; + if (RTEST(since)) { + dc->partial_dump = 1; + dc->since = NUM2SIZET(since); + } else { + dc->partial_dump = 0; + } } static VALUE -dump_result(struct dump_config *dc, VALUE output) +dump_result(struct dump_config *dc) { dump_flush(dc); - if (output == sym_string) { - return rb_str_resurrect(dc->string); - } - else if (output == sym_file) { + if (dc->string) { + return dc->string; + } else { rb_io_flush(dc->stream); return dc->stream; } - else { - return Qnil; - } } -/* - * call-seq: - * ObjectSpace.dump(obj[, output: :string]) # => "{ ... }" - * ObjectSpace.dump(obj, output: :file) # => #<File:/tmp/rubyobj20131125-88733-1xkfmpv.json> - * ObjectSpace.dump(obj, output: :stdout) # => nil - * - * Dump the contents of a ruby object as JSON. - * - * This method is only expected to work with C Ruby. - * This is an experimental method and is subject to change. - * In particular, the function signature and output format are - * not guaranteed to be compatible in future versions of ruby. - */ - static VALUE -objspace_dump(int argc, VALUE *argv, VALUE os) +objspace_dump(VALUE os, VALUE obj, VALUE output) { - static const char filename[] = "rubyobj"; - VALUE obj = Qnil, opts = Qnil, output; struct dump_config dc = {0,}; - - rb_scan_args(argc, argv, "1:", &obj, &opts); - - output = dump_output(&dc, opts, sym_string, filename); + dump_output(&dc, output, Qnil, Qnil); dump_object(obj, &dc); - return dump_result(&dc, output); + return dump_result(&dc); } -/* - * call-seq: - * ObjectSpace.dump_all([output: :file]) # => #<File:/tmp/rubyheap20131125-88469-laoj3v.json> - * ObjectSpace.dump_all(output: :stdout) # => nil - * ObjectSpace.dump_all(output: :string) # => "{...}\n{...}\n..." - * ObjectSpace.dump_all(output: - * File.open('heap.json','w')) # => #<File:heap.json> - * ObjectSpace.dump_all(output: :string, - * since: 42) # => "{...}\n{...}\n..." - * - * Dump the contents of the ruby heap as JSON. - * - * _since_ must be a non-negative integer or +nil+. - * - * If _since_ is a positive integer, only objects of that generation and - * newer generations are dumped. The current generation can be accessed using - * GC::count. - * - * Objects that were allocated without object allocation tracing enabled - * are ignored. See ::trace_object_allocations for more information and - * examples. - * - * If _since_ is omitted or is +nil+, all objects are dumped. - * - * This method is only expected to work with C Ruby. - * This is an experimental method and is subject to change. - * In particular, the function signature and output format are - * not guaranteed to be compatible in future versions of ruby. - */ - static VALUE -objspace_dump_all(int argc, VALUE *argv, VALUE os) +objspace_dump_all(VALUE os, VALUE output, VALUE full, VALUE since) { - static const char filename[] = "rubyheap"; - VALUE opts = Qnil, output; struct dump_config dc = {0,}; - - rb_scan_args(argc, argv, "0:", &opts); - - output = dump_output(&dc, opts, sym_file, filename); + dump_output(&dc, output, full, since); if (!dc.partial_dump || dc.since == 0) { /* dump roots */ @@ -685,7 +602,7 @@ objspace_dump_all(int argc, VALUE *argv, VALUE os) https://github.com/ruby/ruby/blob/trunk/ext/objspace/objspace_dump.c#L602 /* dump all objects */ rb_objspace_each_objects(heap_i, &dc); - return dump_result(&dc, output); + return dump_result(&dc); } void @@ -696,15 +613,8 @@ Init_objspace_dump(VALUE rb_mObjSpace) https://github.com/ruby/ruby/blob/trunk/ext/objspace/objspace_dump.c#L613 rb_mObjSpace = rb_define_module("ObjectSpace"); /* let rdoc know */ #endif - rb_define_module_function(rb_mObjSpace, "dump", objspace_dump, -1); - rb_define_module_function(rb_mObjSpace, "dump_all", objspace_dump_all, -1); - - sym_output = ID2SYM(rb_intern("output")); - sym_stdout = ID2SYM(rb_intern("s (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/