Optimize Symbol to_msgpack_ext and from_msgpack_ext

[casperisfine]

require 'benchmark/ips'
require 'msgpack'

symbol = :foo

original_factory = MessagePack::Factory.new
original_factory.register_type(0x00, Symbol)
original_payload = original_factory.dump(symbol)

optimised_factory = MessagePack::Factory.new
optimised_factory.register_type(0x00, Symbol, packer: Symbol.method_defined?(:name) ? :name.to_proc : :to_s.to_proc, unpacker: :to_sym.to_proc)
optimised_payload = optimised_factory.dump(symbol)

raise "Format change" unless original_payload == optimised_payload

puts "== #{RUBY_DESCRIPTION} =="
puts
puts "-- load --"
Benchmark.ips do |x|
  x.report('original load') { original_factory.load(original_payload) }
  x.report('optimised load') { optimised_factory.load(optimised_payload) }
  x.compare!
end

puts "-- dump --"
Benchmark.ips do |x|
  x.report('original dump') { original_factory.dump(symbol) }
  x.report('optimised dump') { optimised_factory.dump(symbol) }
  x.compare!
end

== ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19] ==

-- load --
Warming up --------------------------------------
       original load    89.559k i/100ms
      optimised load    96.724k i/100ms
Calculating -------------------------------------
       original load    904.759k (± 2.0%) i/s -      4.568M in   5.050463s
      optimised load    976.025k (± 1.6%) i/s -      4.933M in   5.055455s

Comparison:
      optimised load:   976024.9 i/s
       original load:   904758.9 i/s - 1.08x  (± 0.00) slower

-- dump --
Warming up --------------------------------------
       original dump    63.903k i/100ms
      optimised dump    87.429k i/100ms
Calculating -------------------------------------
       original dump    631.579k (± 1.4%) i/s -      3.195M in   5.059979s
      optimised dump    862.071k (± 1.3%) i/s -      4.371M in   5.071797s

Comparison:
      optimised dump:   862071.2 i/s
       original dump:   631579.1 i/s - 1.36x  (± 0.00) slower

== ruby 3.0.0p0 (2021-01-14 revision 42098249b2) [x86_64-darwin19] ==

-- load --
Warming up --------------------------------------
       original load    19.213k i/100ms
      optimised load    24.709k i/100ms
Calculating -------------------------------------
       original load    191.742k (± 1.6%) i/s -    960.650k in   5.011484s
      optimised load    249.776k (± 1.5%) i/s -      1.260M in   5.046264s

Comparison:
      optimised load:   249775.6 i/s
       original load:   191742.5 i/s - 1.30x  (± 0.00) slower

-- dump --
Warming up --------------------------------------
       original dump    14.885k i/100ms
      optimised dump    21.402k i/100ms
Calculating -------------------------------------
       original dump    148.451k (± 1.7%) i/s -    744.250k in   5.014900s
      optimised dump    212.112k (± 1.2%) i/s -      1.070M in   5.045697s

Comparison:
      optimised dump:   212111.6 i/s
       original dump:   148450.9 i/s - 1.43x  (± 0.00) slower

[tagomoris]

This is a completely off-topic question: Why is the benchmark on Ruby 3.0 so slower than Ruby 2.7?

[casperisfine]

Hum, I didn't notice, that's a good question. I'll have a look.

[casperisfine]

Hum, it's probably just my ruby 3.0 that's compiled in debug mode:

require 'benchmark/ips'
str = :foobar.to_s
Benchmark.ips do |x|
  x.report('to_sym') { str.to_sym }
end

$ chruby_use /opt/rubies/3.0.0-pshopify3/
$ ruby /tmp/bench-symbols.rb 
Warming up --------------------------------------
              to_sym   202.948k i/100ms
Calculating -------------------------------------
              to_sym      2.011M (± 1.1%) i/s -     10.147M in   5.046170s
$ chruby_use /opt/rubies/2.7.2/
$ ruby /tmp/bench-symbols.rb 
Warming up --------------------------------------
              to_sym     1.268M i/100ms
Calculating -------------------------------------
              to_sym     12.581M (± 0.9%) i/s -     63.419M in   5.041164s

[tagomoris]

I'm ok to merge this change, but I'm feeling that the original code is also important to show how it should be (without optimization).
@casperisfine How do you feel if I say I want to leave the original code as comments?

[casperisfine]

I'm feeling that the original code is also important to show how it should be

You mean showing how extensions are supposed to use Array#pack and String.unpack? I don't think the Symbol serializer is the best place for that.

This sounds to me that it would be more the job of the README, no? https://github.com/msgpack/msgpack-ruby#extension-types

[tagomoris]

@casperisfine It's not what I meant. My intention was, the code of this change is highly relying on the current MRI's implementation, and it can be mapped to the MessagePack's requirements.
The original code (using pack/unpack) is independent of internal implementation. So, I feel that it may be better to show the original code (as comments) and the newer optimized code in parallel to explain what the optimized code is doing.
Does it make sense?

[casperisfine]

the code of this change is highly relying on the current MRI's implementation

I don't see how. The String <-> Symbol casting with .to_sym / to_s|to_name is standard, and will work the same on Truffle, JRuby etc.

Since MessagePack extensions basically expose strings, I suppose it's fine to directly use theses. Well except for #211 but not sure how we could ever fix that one without breaking backward compat.

SO yeah, not sure I understand your point.

[tagomoris]

MessagePack is a binary serialization format, and I feel we should use pack to get the binary representations of objects, generally speaking.
Of course, I know that we can use string values directly in this case, but also want to show how it can be written using pack/unpack at the same time.

[casperisfine]

As you wish, I'll restore the old code as comment.

[casperisfine]

@tagomoris I've updated the code.

[tagomoris]

@casperisfine Thank you! The added explanation is perfect!
Usually, I strongly hate the behavior to leave older code as comments, but this case was something different to me.

[casperisfine]

Well, it's more like inline documentation in this case so yeah I understand.

[tagomoris]

Merged. Thank you!