Bug #7161

Perf fix: use symbols instead of strings for const/ivar access methods

Added by Charles Nutter over 1 year ago. Updated over 1 year ago.

[ruby-core:47979]
Status:Closed
Priority:Normal
Assignee:Shyouhei Urabe
Category:lib
Target version:-
ruby -v:2.0.0 Backport:

Description

From pull request: https://github.com/ruby/ruby/pull/195

Fixes a number of places where literal, non-dynamic strings are used as the first argument for constant and instance variable get, set, and defined? methods. This reduces object overhead in all cases.

Notable fixes:

  • Setting @originalfilename and @contenttype in read_multipart (cgi/core.rb). This is called for every multipart read.
  • Setting @uri and @ref in DrbObject.new_with (drb/drb.rb). Called for every "with" instantiation of a DrbObject.
  • Setting BINDING_QUEUE in IRB::Workspace#initialize (irb/workspace.rb). Called for every new IRB workspace.
  • Getting @mon_mutex in ConditionVariable#wait (monitor.rb). Called for every #wait. This fix makes the method require zero object allocation (other than imposed by the runtime).

The other fixes are rarely called, but fixed for consistency.

make test-all passes the same with or without this patch.

Associated revisions

Revision 37688
Added by Marc-Andre Lafortune over 1 year ago

  • lib/cgi/core.rb: Use symbols instead of strings for
    {const,instancevariable_}{get,set}. [#7161]

  • lib/drb/drb.rb: ditto.

  • lib/ipaddr.rb: ditto.

  • lib/irb/workspace.rb: ditto.

  • lib/monitor.rb: ditto.

  • lib/rss/maker/base.rb: ditto.

  • lib/rss/rss.rb: ditto.

  • lib/xmlrpc/parser.rb: ditto.

History

#1 Updated by Yusuke Endoh over 1 year ago

  • Status changed from Open to Assigned
  • Assignee set to Shyouhei Urabe

The patch looks benign. I wonder how much is the performance improved, though.
Urabe-san, could you handle the pull request?

Yusuke Endoh mame@tsg.ne.jp

#2 Updated by Charles Nutter over 1 year ago

The performance gain is not tremendous, but it does avoid a lot of intermediate string objects that could cumulatively impact performance through GC overhead.

In any case, it's a good cleanup patch, and there are certainly small perf gains.

#3 Updated by Charles Nutter over 1 year ago

Ping!

#4 Updated by Marc-Andre Lafortune over 1 year ago

  • Category set to lib
  • Status changed from Assigned to Closed

Committed as r37688.

I'm sorry, I should have added 'patch by Charles Nutter' to the commit log but I forgot that although git can track authors, svn does not.

#5 Updated by Charles Nutter over 1 year ago

I'll forgive you this time, Marc-Andre :)

Also available in: Atom PDF