Patch #10978 review request

1 view
Skip to first unread message

Alexander Dymo

unread,
Feb 1, 2008, 1:57:45 PM2/1/08
to rubyonra...@googlegroups.com
Hi,
I've submitted the patch to speedup AR::Base::attributes_with_quotes (and
thus, AR::Base::create and AR::Base::update).

The idea is that #attributes_with_quotes clones attributes which not only
takes some time, but also allocates quite some memory.

My measurements show that my rails application during one of lengthy requests
spends 7 extra seconds and allocates (by malloc's) 650 megabytes of memory in
#attributes_with_quotes call (the blog post explaining in details how the
memory consumption was measured and which request tested is at:
http://blog.pluron.com/2008/01/guerrillas-way.html). Of course that memory is
freed by GC, but that takes quite some time to do.

Could somebody take a look at that patch? Is it safe enough to not clone
attributes in this case?

Rick Olson

unread,
Feb 1, 2008, 3:03:20 PM2/1/08
to rubyonra...@googlegroups.com

Being the genius I am, I couldn't find your ticket so I transcribed
your guerilla monkey patch to edge rails. I went to link up this very
topic in the ticket and finally saw that the patch number was in the
subject. Both patches seem to work fine. This is great work, and I'm
personally all for it...

--
Rick Olson
http://lighthouseapp.com
http://weblog.techno-weenie.net
http://mephistoblog.com

Michael Koziarski

unread,
Feb 1, 2008, 6:01:34 PM2/1/08
to rubyonra...@googlegroups.com
> Being the genius I am, I couldn't find your ticket so I transcribed
> your guerilla monkey patch to edge rails. I went to link up this very
> topic in the ticket and finally saw that the patch number was in the
> subject. Both patches seem to work fine. This is great work, and I'm
> personally all for it...

Yeah, I definitely can't see any downsides for these two patches.
Really nice work! Keep them coming ;)

As an aside, cloning on Base#attributes is probably unnecessary, if
someone feels like removing the cloning and seeing what breaks,
that'd be cool.

--
Cheers

Koz

Reply all
Reply to author
Forward
0 new messages