Guard a wrapped read with a flag on the record

.rubocop.yml

@@ -4,7 +4,7 @@ plugins: rubocop-yard
 # the assertions. Long lines carrying a `#=>` expectation are allowed.
 Layout/LineLength:
   AllowedPatterns:
-    - '#=>'
+    - "#=>"
 
 # Some(), None(), Ok() and Err() are the library's Rust-style value
 # constructors; their capitalized names are the point.
@@ -19,6 +19,7 @@ Metrics/ClassLength:
     - lib/errgonomic/option.rb
     - lib/errgonomic/result.rb
     - test/**/*
+    - benchmark/**/*
 
 # core_ext vendors ActiveSupport's blank?/present? patches; the reopened core
 # classes there are explained by the file header, not per class. Test
@@ -29,21 +30,36 @@ Style/Documentation:
     - test/**/*
 
 # A test states one behavior end to end, and splitting one to satisfy a size
-# metric hides the behavior it was written to name. A cop that names its own
-# Exclude replaces this one, so Metrics/ClassLength repeats the path above.
+# metric hides the behavior it was written to name. Size cops do not apply
+# under test/ or benchmark/. A cop that names its own Exclude replaces this
+# one, so those repeat the two paths.
 Metrics:
   Exclude:
     - test/**/*
+    - benchmark/**/*
 
 # The ActiveRecord hooks in the optional concern are one subject — where a
-# reader may come from and what leaves it alone — and reading them together is
-# the point. The generated reader is a heredoc, which the length cops count as
-# if it were code.
+# reader may come from and what leaves it alone — and reading them together
+# is the point. The generated reader is a heredoc, which the length cops
+# count as if it were code.
 Metrics/BlockLength:
   Exclude:
     - lib/errgonomic/rails/active_record_optional.rb
     - test/**/*
+    - benchmark/**/*
 Metrics/MethodLength:
   Exclude:
     - lib/errgonomic/rails/active_record_optional.rb
     - test/**/*
+    - benchmark/**/*
+
+# has_one overrides ActiveRecord's association macro; it is not a predicate.
+Naming/PredicatePrefix:
+  AllowedMethods:
+    - has_one
+
+# A benchmark prints a table, where the format tokens are positional by
+# nature and naming each one only adds noise.
+Style/FormatStringToken:
+  Exclude:
+    - benchmark/**/*

Rakefile

@@ -18,6 +18,13 @@ end
 
 task default: %i[test yard:doctest]
 
+namespace :benchmark do
+  desc 'Cost of a wrapped attribute read, against the plain reader it replaces'
+  task :optional_reader do
+    ruby '-Ilib benchmark/optional_reader.rb'
+  end
+end
+
 namespace :gems4nix do
   desc 'Regenerate gem-groups.json after Gemfile/Gemfile.lock changes'
   task :groups do

benchmark/optional_reader.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+# What a wrapped attribute read costs, against the plain reader it replaces.
+#
+# Every converted model pays this on every read of a nullable column, so the
+# reader is the one place in the library where a few nanoseconds matter. Two
+# figures are reported per shape: wall time per read, and objects allocated
+# per read, measured as the marginal cost of a second batch so the harness
+# itself cancels out.
+#
+#   rake benchmark:optional_reader
+
+require 'active_record'
+require 'benchmark'
+require 'logger'
+
+require_relative '../lib/errgonomic/rails'
+
+ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
+ActiveRecord::Base.logger = Logger.new(File::NULL)
+
+ActiveRecord::Schema.verbose = false
+ActiveRecord::Schema.define do
+  create_table 'readings', force: :cascade do |t|
+    t.string :note
+  end
+end
+
+Errgonomic::Rails.setup_before
+
+# The reader ActiveRecord would have given us, as the floor.
+class PlainReading < ActiveRecord::Base
+  self.table_name = 'readings'
+end
+
+# The reader the concern generates, guard and all.
+class WrappedReading < ActiveRecord::Base
+  self.table_name = 'readings'
+  include Errgonomic::Rails::ActiveRecordOptional
+end
+
+# Wrapping with no guard at all, as the ceiling on what the guard may cost:
+# the difference between this and WrappedReading is the guard's price.
+class UnguardedReading < ActiveRecord::Base
+  self.table_name = 'readings'
+
+  def note
+    val = super
+    val.nil? ? Errgonomic::Option::None.new : Errgonomic::Option::Some.new(val)
+  end
+end
+
+SHAPES = { 'plain' => PlainReading, 'wrapped' => WrappedReading, 'unguarded' => UnguardedReading }.freeze
+READS = 200_000
+
+def nanoseconds_per_read(record)
+  record.note
+  Benchmark.realtime { READS.times { record.note } } / READS * 1e9
+end
+
+def allocations_per_read(record)
+  record.note
+  before = GC.stat(:total_allocated_objects)
+  READS.times { record.note }
+  one_batch = GC.stat(:total_allocated_objects)
+  (READS * 2).times { record.note }
+  two_batches = GC.stat(:total_allocated_objects) - one_batch
+  (two_batches - (one_batch - before)).fdiv(READS)
+end
+
+records = SHAPES.transform_values { |klass| klass.new(note: 'present') }
+baseline = nanoseconds_per_read(records.fetch('plain'))
+
+puts format('%-10s %10s %10s %14s', 'shape', 'ns/read', 'vs plain', 'allocs/read')
+records.each do |name, record|
+  nanoseconds = nanoseconds_per_read(record)
+  puts format('%-10s %10.0f %9.1fx %14.1f', name, nanoseconds, nanoseconds / baseline,
+              allocations_per_read(record))
+end

lib/errgonomic/rails/active_record_optional.rb

@@ -168,6 +168,11 @@ def errgonomic_unwrap_optionals(*names)
           end
         end
 
+        # The reader guards against re-entering itself with a flag on the
+        # record. Bookkeeping shared across records would have to build a key
+        # per read, and this reader is on the hot path of every wrapped
+        # attribute. A record read from two threads at once is out of scope,
+        # as it is for ActiveRecord itself.
         def errgonomic_wrap_optional(name)
           name = name.to_s
           return if errgonomic_optional_off?
@@ -176,18 +181,20 @@ def errgonomic_wrap_optional(name)
           errgonomic_optional_names << name
           class_eval <<-RUBY, __FILE__, __LINE__ + 1
             def #{name}
-              reads = Thread.current[:errgonomic_optional_reads] ||= {}
-              key = [object_id, :#{name}]
-              if reads[key]
-                raise Errgonomic::RecursiveOptionalReadError,
-                      "\#{self.class}##{name} re-entered itself; something beneath this reader reads it again"
+              if @__errgonomic_reading_#{name}
+                raise Errgonomic::RecursiveOptionalReadError, <<~MSG
+                  \#{self.class}##{name} re-entered itself; something beneath this reader reads it again.
+                  If this read was not recursive, the record may have been read from two threads at once,
+                  which this guard cannot tell apart. Please report that at
+                  https://github.com/omc/errgonomic/issues
+                MSG
               end
 
-              reads[key] = true
+              @__errgonomic_reading_#{name} = true
               begin
                 val = super
               ensure
-                reads.delete(key)
+                @__errgonomic_reading_#{name} = false
               end
               val.nil? ? Errgonomic::Option::None.new : Errgonomic::Option::Some.new(val)
             end

test/rails_test.rb

@@ -305,6 +305,25 @@ def test_optional_attributes_read_on_a_deep_stack
     deeper(1100) { assert_equal 'writes sci-fi', author.bio.unwrap! }
   end
 
+  # A wrapped read sits on the hot path of every converted model, so the only
+  # allocation it may make is the Option it returns.
+  def test_a_wrapped_read_allocates_only_the_option_it_returns
+    author = Author.create!(name: 'Cixin Liu', bio: 'writes sci-fi')
+    author.bio
+
+    # Difference two runs so the measurement itself cancels out and only the
+    # per-read allocation is left.
+    before = GC.stat(:total_allocated_objects)
+    100.times { author.bio }
+    hundred = GC.stat(:total_allocated_objects)
+    200.times { author.bio }
+    three_hundred = GC.stat(:total_allocated_objects)
+
+    assert_equal 100, (three_hundred - hundred) - (hundred - before)
+  end
+
+  # A read that re-enters itself still has to say so at the first re-entry,
+  # naming the reader, rather than unwinding as a stack overflow.
   def test_recursive_read_raises_a_named_error_at_first_reentry
     author = LoopyAuthor.create!(name: 'Cixin Liu', bio: 'writes sci-fi')
     error = assert_raises(Errgonomic::RecursiveOptionalReadError) { author.bio }