♻️ Refactor SequenceSet#dup, #clone, and #replace

This was originally part of some performance improvements.  But any
speedup reported by the (newly added) benchmark isn't significant.  So
this change is more for the semantics of these methods.

Changes:
* don't delegate `#initialize_clone` and `#replace` to `#initialize_dup`
* call `#modifying!` and copy the string in `#replace`
* don't call `#modifying!` in `#initialize_dup` or `#initialize_clone`
* don't copy or build the string in `#initialize_dup`.
  `@string` is always frozen, so the default shallow copy is enough.

Author: nevans

benchmarks/sequence_set-copy.yml

@@ -0,0 +1,96 @@
+---
+prelude: |
+  require "yaml"
+  require "net/imap"
+
+  INPUT_COUNT = Integer ENV.fetch("BENCHMARK_INPUT_COUNT", 1000)
+  MAX_INPUT   = Integer ENV.fetch("BENCHMARK_MAX_INPUT",   1400)
+  WARMUP_RUNS = Integer ENV.fetch("BENCHMARK_WARMUP_RUNS", 1000)
+
+  require "./test/lib/profiling_helper"
+  include ProfilingHelper
+
+  def init_sets(count: 100, set_size: INPUT_COUNT, max: MAX_INPUT)
+    Array.new(count) {
+      Net::IMAP::SequenceSet.new(Array.new(set_size) { rand(1..max) })
+    }
+  end
+
+  def init_normal_sets(...)
+    init_sets(...)
+  end
+
+  def init_unsorted_sets(...)
+    init_sets(...)
+      .each do |seqset|
+        entries = seqset.entries.shuffle
+        seqset.clear
+        entries.each do |entry|
+          seqset.append entry
+        end
+      end
+  end
+
+  # warmup (esp. for JIT)
+  WARMUP_RUNS.times do
+    init_sets(count: 20, set_size: 100, max: 120).each do |set|
+      set.dup
+      set.clone
+      Net::IMAP::SequenceSet.new set
+    end
+  end
+
+benchmark:
+  - name: "normal.dup (#string not called)"
+    prelude: $sets = init_normal_sets
+    script:  $sets.sample.dup
+  - name: "normal.dup (#string called)"
+    prelude: $sets = init_normal_sets.tap do _1.each(&:string) end
+    script:  $sets.sample.dup
+  - name: "normal.freeze.dup"
+    prelude: $sets = init_normal_sets
+    script:  $sets.sample.freeze.dup
+  - name: "frozen.dup"
+    prelude: $sets = init_normal_sets.map(&:freeze)
+    script:  $sets.sample.dup
+
+  - name: "normal.clone (#string not called)"
+    prelude: $sets = init_normal_sets
+    script:  $sets.sample.clone
+  - name: "normal.clone (#string called)"
+    prelude: $sets = init_normal_sets.tap do _1.each(&:string) end
+    script:  $sets.sample.clone
+  - name: "normal.freeze.clone"
+    prelude: $sets = init_normal_sets
+    script:  $sets.sample.freeze.clone
+  - name: "frozen.clone"
+    prelude: $sets = init_normal_sets.map(&:freeze)
+    script:  $sets.sample.clone
+
+  - name: "SequenceSet.new(normal) (#string not called)"
+    prelude: $sets = init_normal_sets
+    script:  Net::IMAP::SequenceSet.new $sets.sample
+  - name: "SequenceSet.new(normal) (#string called)"
+    prelude: $sets = init_normal_sets.tap do _1.each(&:string) end
+    script:  Net::IMAP::SequenceSet.new $sets.sample
+  - name: "SequenceSet.new(frozen)"
+    prelude: $sets = init_normal_sets.map(&:freeze)
+    script:  Net::IMAP::SequenceSet.new $sets.sample
+  - name: "SequenceSet.new(unsorted)"
+    prelude: $sets = init_unsorted_sets
+    script:  Net::IMAP::SequenceSet.new $sets.sample
+
+contexts:
+  - name: local
+    prelude: |
+      $LOAD_PATH.unshift "./lib"
+      $allowed_to_profile = true # only profile local code
+    require: false
+  - name: v0.5.9
+    gems:
+      net-imap: 0.5.9
+    require: false
+  - name: v0.4.21
+    gems:
+      net-imap: 0.4.21
+    require: false

lib/net/imap/sequence_set.rb

@@ -547,7 +547,10 @@ def clear
       # accepted by ::new.
       def replace(other)
         case other
-        when SequenceSet then initialize_dup(other)
+        when SequenceSet then
+          modifying! # short circuit before doing any work
+          @tuples = other.deep_copy_tuples
+          @string = other.instance_variable_get(:@string)
         when String      then self.string = other
         else                  clear; merge other
         end
@@ -1734,20 +1737,21 @@ def init_with(coder) # :nodoc:
 
       attr_reader :tuples # :nodoc:
 
+      def deep_copy_tuples; @tuples.map { _1.dup } end # :nodoc:
+
       private
 
       def remain_frozen(set) frozen? ? set.freeze : set end
       def remain_frozen_empty; frozen? ? SequenceSet.empty : SequenceSet.new end
 
       # frozen clones are shallow copied
       def initialize_clone(other)
-        other.frozen? ? super : initialize_dup(other)
+        @tuples = other.deep_copy_tuples unless other.frozen?
+        super
       end
 
       def initialize_dup(other)
-        modifying! # redundant check, to normalize the error message for JRuby
-        @tuples = other.tuples.map(&:dup)
-        @string = other.string&.-@
+        @tuples = other.deep_copy_tuples
         super
       end