On Thursday, December 2, 2004, 5:13:37 PM, Eric wrote:
> I have a small testcase in rails that has 7 tests in it (4 of them
> raise NotImplementedError), but simply starting up the testcase takes 3
> seconds:
> $ ruby -r profile -Ilib:test test/unit/emailer_test.rb
> % cumulative self self total
> time seconds seconds calls ms/call ms/call name
> 18.37 5.57 5.57 462 12.06 19.42 Integer#gcd
> 10.26 8.68 3.11 4008 0.78 2.37 Gem::Specification#copy_of
I'm looking into this now, Eric. 4008 is a lot of calls. See bottom
of email for a patch you can try to see if it gets reduced.
> [...] Is all the copying done in #copy_of completely necessary?
Well, that is the question. find, xargs, and grep tell me that
copy_of appears in the following places only:
specification.rb:124: @#{name} ||= copy_of(@@default_value[:#{name}])
specification.rb:350: self.send "#{name}=", copy_of(default)
specification.rb:567: def self.copy_of(obj)
specification.rb:575: def copy_of(obj)
specification.rb:576: self.class.copy_of(obj)
The first two are invocations; the rest are defintions, or part
thereof.
The first one appears in this context:
def self.attribute(name, default=nil)
@@attributes << [name, default]
@@default_value[name] = default
attr_writer(name)
class_eval %{
def #{name}
@#{name} ||= copy_of(@@default_value[:#{name}])
end
}
end
This is to allow the specification of default attribute values.
class Specification
attribute :bindir, 'bin'
attribute :files, []
...
end
The comment above the method is:
##
# Used to specify the name and default value of a specification attribute. The side
# effects are:
# * the name and default value are added to the @@attributes list and
# @@default_value map
# * a standard _writer_ method (<tt>attribute=</tt>) is created
# * a non-standard _reader method (<tt>attribute</tt>) is created
#
# The reader method behaves like this:
# def attribute
# @attribute ||= (copy of default value)
# end
#
# This allows lazy initialization of attributes to their default values.
#
So attribute reader methods are generated. The code corresponding to
our two attributes 'bindir' and 'files' above is:
def bindir
@bindir ||= copy_of(@@default_value[:bindir])
# @@default_value[:bindir] == 'bin'
end
def files
@files ||= copy_of(@@default_value[:files])
# @@default_value[:files] == []
end
Do you see the need for the copy_of? The default value for the
attributes is held in a class variable. The reader lazily sets the
instance variable to the default value. We must take a copy of the
default value, otherwise every Specification object will be pointing
to the same array of files, for instance.
Now, let's challenge some of the thinking behind all this.
Q. Why all this metaprogramming?
A. To allow Gem::Specification to be defined in a non-redundant
manner, avoiding inconsistencies which are ugly and often lead to
bugs. Also, it exposes the names and defaults of attributes
programmatically, which can help with documentation.
Q. But doesn't it slow things down?
A. The impact of the metaprogramming should only be felt when the
class is defined. This performance concern with #copy_of demonstrates
that it impacts attribute readers as well, which we should try to fix.
Q. Why perform lazy initialisation of attributes? Why not just
initialize all instance variables up front?
A. Good question. It's to avoid creating lots of instance variables
that aren't necessarily used. The motive for cutting down on instance
variables is to reduce the amount of YAML output. YAML reduction can
be achieved in other -- probably better -- ways, so we should see if
we can get a performance boost from up-front initialisation.
It so happens that up-front initialisation _does_ occur with this bit
of #initialize:
@@attributes.each do |name, default|
self.send "#{name}=", copy_of(default)
end
I'll investigate whether this apparent duplication is an oversight.
It seems that way. If so, the "smart" attribute reader methods can
become dumb ones, since we're creating all the instance variables
anyway. That should reduce the number of calls to copy_of.
Eric, if you're still reading, you can help investigate this issue by
applying a simple patch (at bottom of email) against the latest
specification.rb (which shipped with RubyGems 0.8.3). Run your tests
again, and see how many calls are made to #copy_of, and see if
anything breaks.
This is an investigative patch only, so you should be careful to
restore the original afterwards.
Cheers,
Gavin
Index: specification.rb
===================================================================
RCS file: /var/cvs/rubygems/rubygems/lib/rubygems/specification.rb,v
retrieving revision 1.65
diff -u -r1.65 specification.rb
--- specification.rb 7 Dec 2004 04:01:05 -0000 1.65
+++ specification.rb 10 Dec 2004 01:42:03 -0000
@@ -124,12 +124,7 @@
def self.attribute(name, default=nil)
@@attributes << [name, default]
@@default_value[name] = default
- attr_writer(name)
- class_eval %{
- def #{name}
- @#{name} ||= copy_of(@@default_value[:#{name}])
- end
- }
+ attr_accessor(name)
end
# Same as attribute above, but also records this attribute as mandatory.