Avoiding clone in AR::Base#attributes

Hi,
I’ve changed the ActiveRecord::Base#attributes method to avoid cloning of objects and increase apps performance.
Here is the patch: http://dev.rubyonrails.org/ticket/11047

This sure speeds up the method avoiding Kernel#clone calls, but I don’t know if there is any special case where not cloning makes this unsafe.
As koz suggested ( http://groups.google.com/group/rubyonrails-core/browse_thread/thread/faeaed67dee0ff1d/c0b0c81e9a4f6eb3
) I’ve could not find cloning here being necessary and no test is broken.
Any doublecheck, comment or +/-1 is welcome.

Cheers,
Juanjo

Hi,
I've changed the ActiveRecord::Base#attributes method to avoid cloning of
objects and increase apps performance.
Here is the patch: http://dev.rubyonrails.org/ticket/11047
This sure speeds up the method avoiding Kernel#clone calls, but I don't
know if there is any special case where not cloning makes this unsafe.
As koz suggested (
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/faeaed67dee0ff1d/c0b0c81e9a4f6eb3
) I've could not find cloning here being necessary and no test is broken.
Any doublecheck, comment or +/-1 is welcome.

The only case that breaks (that I can think of) is where you assign
the attributes of one model to another, they'll end up sharing the
same instances for their attributes. For example:

  def test_values_copied_with_attributes_assignment
    new_topic = Topic.create(:title=>"Some title")
    other_topic = Topic.create(new_topic.attributes)
    assert_equal new_topic.title, other_topic.title
    new_topic.title.gsub!(/Some/, 'The')
    assert_equal "The title", new_topic.title
    assert_equal "Some title", other_topic.title
  end

With your patch that test would fail, both topics would have a title
of 'The title'. I'm not particularly worried about this, but it
would be best to leave it till 2.1 rather than a 2.0 point release.

Anyone have any objections to this new behaviour?

Anyone have any objections to this new behaviour?

I've applied the patch to edge, so if you notice any breakage shout
out in this thread.

As nobody reports any breakage, it seems natural to implement the same behaviour to attributes_before_type_cast as well.

Here is the patch to avoid cloning in attributes_before_type_cast: http://dev.rubyonrails.org/ticket/11077

Cheers,
Juanjo

As nobody reports any breakage, it seems natural to implement the same
behaviour to attributes_before_type_cast as well.

Here is the patch to avoid cloning in attributes_before_type_cast:
http://dev.rubyonrails.org/ticket/11077

Agreed, applied, thanks!