From c5a445d577b786e31c420b5857ad05b954b420ec Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Sun, 20 Dec 2020 21:42:25 -0500 Subject: [PATCH] [ruby/psych] Make Ractor-ready. Config is Ractor-local. Benchmarking reveals that using `Ractor.local_storage` for storing cache is similar to accessing a constant (~15% slower). --- ext/psych/lib/psych.rb | 26 ++++++++++++-- ext/psych/lib/psych/visitors/visitor.rb | 20 +++++++++-- ext/psych/psych.c | 3 ++ test/psych/test_ractor.rb | 47 +++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 test/psych/test_ractor.rb diff --git a/ext/psych/lib/psych.rb b/ext/psych/lib/psych.rb index 0b62362198..cedf0a4ad6 100644 --- a/ext/psych/lib/psych.rb +++ b/ext/psych/lib/psych.rb @@ -632,9 +632,29 @@ module Psych private_class_method :warn_with_uplevel, :parse_caller class << self - attr_accessor :load_tags - attr_accessor :dump_tags - attr_accessor :domain_types + if defined?(Ractor) + require 'forwardable' + extend Forwardable + + class Config + attr_accessor :load_tags, :dump_tags, :domain_types + def initialize + @load_tags = {} + @dump_tags = {} + @domain_types = {} + end + end + + def config + Ractor.current[:PsychConfig] ||= Config.new + end + + def_delegators :config, :load_tags, :dump_tags, :domain_types, :load_tags=, :dump_tags=, :domain_types= + else + attr_accessor :load_tags + attr_accessor :dump_tags + attr_accessor :domain_types + end end self.load_tags = {} self.dump_tags = {} diff --git a/ext/psych/lib/psych/visitors/visitor.rb b/ext/psych/lib/psych/visitors/visitor.rb index 3f4ba64e57..35f8f81f18 100644 --- a/ext/psych/lib/psych/visitors/visitor.rb +++ b/ext/psych/lib/psych/visitors/visitor.rb @@ -8,12 +8,26 @@ module Psych private - DISPATCH = Hash.new do |hash, klass| - hash[klass] = "visit_#{klass.name.gsub('::', '_')}" + # @api private + def self.dispatch_cache + Hash.new do |hash, klass| + hash[klass] = :"visit_#{klass.name.gsub('::', '_')}" + end + end + + if defined?(Ractor) + def dispatch + Ractor.current[:Psych_Visitors_Visitor] ||= Visitor.dispatch_cache + end + else + DISPATCH = dispatch_cache + def dispatch + DISPATCH + end end def visit target - send DISPATCH[target.class], target + send dispatch[target.class], target end end end diff --git a/ext/psych/psych.c b/ext/psych/psych.c index 0afd7318b5..8af0bb6a5a 100644 --- a/ext/psych/psych.c +++ b/ext/psych/psych.c @@ -22,6 +22,9 @@ VALUE mPsych; void Init_psych(void) { + #ifdef HAVE_RB_EXT_RACTOR_SAFE + RB_EXT_RACTOR_SAFE(true); + #endif mPsych = rb_define_module("Psych"); rb_define_singleton_method(mPsych, "libyaml_version", libyaml_version, 0); diff --git a/test/psych/test_ractor.rb b/test/psych/test_ractor.rb new file mode 100644 index 0000000000..90e40360c0 --- /dev/null +++ b/test/psych/test_ractor.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true +require_relative 'helper' + +class TestPsychRactor < Test::Unit::TestCase + def test_ractor_round_trip + assert_ractor(<<~RUBY, require_relative: 'helper') + obj = {foo: [42]} + obj2 = Ractor.new(obj) do |obj| + Psych.load(Psych.dump(obj)) + end.take + assert_equal obj, obj2 + RUBY + end + + def test_not_shareable + # There's no point in making these frozen / shareable + # and the C-ext disregards begin frozen + assert_ractor(<<~RUBY, require_relative: 'helper') + parser = Psych::Parser.new + emitter = Psych::Emitter.new(nil) + assert_raise(Ractor::Error) { Ractor.make_shareable(parser) } + assert_raise(Ractor::Error) { Ractor.make_shareable(emitter) } + RUBY + end + + def test_ractor_config + assert_ractor(<<~RUBY, require_relative: 'helper') + r = Ractor.new do + Psych.add_builtin_type 'omap' do |type, val| + val * 2 + end + Psych.load('--- !!omap hello') + end.take + assert_equal 'hellohello', r + assert_equal 'hello', Psych.load('--- !!omap hello') + RUBY + end + + def test_ractor_constants + assert_ractor(<<~RUBY, require_relative: 'helper') + r = Ractor.new do + Psych.libyaml_version.join('.') == Psych::LIBYAML_VERSION + end.take + assert_equal true, r + RUBY + end +end