From: eregontp@...
Date: 2017-03-24T16:48:09+00:00
Subject: [ruby-core:80314] [Ruby trunk Bug#13358] OpenStruct overriding	allocate

Issue #13358 has been updated by Eregon (Benoit Daloze).


nobu (Nobuyoshi Nakada) wrote:
> For what purpose?
> It should be used by `Class#new`, `Kernel#dup`, `Kernel#clone`, and `Marshal.load`.

But not only, after all it is a public and well-known method (many hits on GitHub code search).
Many deserializing libraries will use it,
but also various libraries which need to build an instance without passing in state,
or use it as a way to replicate Class#new without using super like (maybe to use a different initializing method):

~~~ ruby
def self.new(name)
  if r = CACHE[name]
    r
  else
    # Could use super(name), but not everybody does
    allocate.tap { |obj| obj.send(:initialize, name) }
  end
end
~~~

> As it is not called usually, it should take the performance penalty, not `respond_to?`.

I think respond_to? should work and not throw an exception on any Object,
even if not fully initialized, just like say #instance_variable_defined?, #equal? and #object_id.

The performance hit on respond_to? is not significant, it's just an extra NIL_P.
On the other hand, the one on allocate is, and affects every caller of OpenStruct.allocate.

~~~ ruby
require 'benchmark/ips'
require 'ostruct'

class SomeClass
end

Benchmark.ips do |x|
  x.report("OpenStruct.allocate") { OpenStruct.allocate }
  x.report("Class#allocate") { SomeClass.allocate }
  x.compare!
end

class MyOpenStruct
  def respond_to_missing?(mid, include_private = false) # :nodoc:
    mname = mid.to_s.chomp("=").to_sym
    @table&.key?(mname) || super
  end
end

os = OpenStruct.new
my = MyOpenStruct.new

Benchmark.ips do |x|
  x.report("OpenStruct#respond_to?") { os.respond_to?(:init_with?) }
  x.report("MyOpenStruct#respond_to?") { my.respond_to?(:init_with?) }
  x.compare!
end
~~~

~~~
Comparison:
      Class#allocate:  7783868.7 i/s
 OpenStruct.allocate:  3813277.6 i/s - 2.04x  slower
Comparison:
  OpenStruct#respond_to?:  1752400.3 i/s
MyOpenStruct#respond_to?:  1734649.5 i/s - same-ish: difference falls within error
~~~

@nobu: Can I commit my fix?
It is faster and does not change the semantics of OpenStruct.allocate.

----------------------------------------
Bug #13358: OpenStruct overriding allocate
https://bugs.ruby-lang.org/issues/13358#change-63785

* Author: sitter (Harald Sitter)
* Status: Closed
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-linux]
* Backport: 2.2: DONTNEED, 2.3: REQUIRED, 2.4: DONTNEED
----------------------------------------
In https://github.com/ruby/ruby/commit/15960b37e82ba60455c480b1c23e1567255d3e05 OpenStruct gained

~~~ruby
  class << self # :nodoc:
    alias allocate new
  end
~~~

Which is rather severely conflicting with expected behavior as `Class.allocate` is meant to [not call initialize](http://ruby-doc.org/core-2.4.0/Class.html#method-i-allocate). So, in fact, the change made `allocate` of `OpenStruct` do what `allocate` is asserting not to do :-/

For `OpenStruct` itself that isn't that big a deal, for classes inheriting from `OpenStruct` it breaks `allocate` though.

Example:

~~~ruby
require 'ostruct'

class A < OpenStruct
  def initialize(x, y = {})
    super(y)
  end
end

A.allocate
~~~

As `allocate` is alias'd to `new` in `OpenStruct` this will attempt to initialize `A` which will raise an `ArgumentError` because `A` cannot be initialized without arguments.

~~~
$ ruby x.rb
x.rb:4:in `initialize': wrong number of arguments (given 0, expected 1..2) (ArgumentError)
        from x.rb:9:in `new'
        from x.rb:9:in `<main>'
~~~

OpenStruct at the very least should document the fact that its allocate is behaving differently.
Ideally, `OpenStruct` should not alias allocate at all.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>