Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Use symbols instead of strings for {const_,instance_variable_}{get,set}. #195

Closed
wants to merge 1 commit into from

2 participants

@headius

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 @original_filename and @content_type 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.

@headius headius referenced this pull request from a commit in jruby/ruby
@headius headius Use JSE properties for proxy defaults (JRuby GH #195) a4ecbbb
@marcandre
Collaborator

Committed as r37688.

Thanks for the PR.

@marcandre marcandre closed this
@marcandre
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
4 lib/cgi/core.rb
@@ -530,11 +530,11 @@ def local_path
/Content-Disposition:.* filename=(?:"(.*?)"|([^;\r\n]*))/i.match(head)
filename = $1 || $2 || ''
filename = CGI.unescape(filename) if unescape_filename?()
- body.instance_variable_set('@original_filename', filename.taint)
+ body.instance_variable_set(:@original_filename, filename.taint)
## content type
/Content-Type: (.*)/i.match(head)
(content_type = $1 || '').chomp!
- body.instance_variable_set('@content_type', content_type.taint)
+ body.instance_variable_set(:@content_type, content_type.taint)
## query parameter name
/Content-Disposition:.* name=(?:"(.*?)"|([^;\r\n]*))/i.match(head)
name = $1 || $2 || ''
View
4 lib/drb/drb.rb
@@ -1010,8 +1010,8 @@ def self._load(s)
def self.new_with(uri, ref)
it = self.allocate
- it.instance_variable_set('@uri', uri)
- it.instance_variable_set('@ref', ref)
+ it.instance_variable_set(:@uri, uri)
+ it.instance_variable_set(:@ref, ref)
it
end
View
2  lib/ipaddr.rb
@@ -607,7 +607,7 @@ def _to_string(addr)
end
-unless Socket.const_defined? "AF_INET6"
+unless Socket.const_defined? :AF_INET6
class Socket < BasicSocket
# IPv6 protocol family
AF_INET6 = Object.new
View
2  lib/irb/workspace.rb
@@ -38,7 +38,7 @@ def initialize(*main)
unless defined? BINDING_QUEUE
require "thread"
- IRB.const_set("BINDING_QUEUE", SizedQueue.new(1))
+ IRB.const_set(:BINDING_QUEUE, SizedQueue.new(1))
Thread.abort_on_exception = true
Thread.start do
eval "require \"irb/ws-for-case-2\"", TOPLEVEL_BINDING, __FILE__, __LINE__
View
2  lib/monitor.rb
@@ -107,7 +107,7 @@ def wait(timeout = nil)
@monitor.__send__(:mon_check_owner)
count = @monitor.__send__(:mon_exit_for_cond)
begin
- @cond.wait(@monitor.instance_variable_get("@mon_mutex"), timeout)
+ @cond.wait(@monitor.instance_variable_get(:@mon_mutex), timeout)
return true
ensure
@monitor.__send__(:mon_enter_for_cond, count)
View
4 lib/rss/maker/base.rb
@@ -23,8 +23,8 @@ def inherited_base
end
def inherited(subclass)
- subclass.const_set("OTHER_ELEMENTS", [])
- subclass.const_set("NEED_INITIALIZE_VARIABLES", [])
+ subclass.const_set(:OTHER_ELEMENTS, [])
+ subclass.const_set(:NEED_INITIALIZE_VARIABLES, [])
end
def add_other_element(variable_name)
View
18 lib/rss/rss.rb
@@ -701,18 +701,18 @@ def inherited_base
end
def inherited(klass)
- klass.const_set("MUST_CALL_VALIDATORS", {})
- klass.const_set("MODELS", [])
- klass.const_set("GET_ATTRIBUTES", [])
- klass.const_set("HAVE_CHILDREN_ELEMENTS", [])
- klass.const_set("TO_ELEMENT_METHODS", [])
- klass.const_set("NEED_INITIALIZE_VARIABLES", [])
- klass.const_set("PLURAL_FORMS", {})
+ klass.const_set(:MUST_CALL_VALIDATORS, {})
+ klass.const_set(:MODELS, [])
+ klass.const_set(:GET_ATTRIBUTES, [])
+ klass.const_set(:HAVE_CHILDREN_ELEMENTS, [])
+ klass.const_set(:TO_ELEMENT_METHODS, [])
+ klass.const_set(:NEED_INITIALIZE_VARIABLES, [])
+ klass.const_set(:PLURAL_FORMS, {})
tag_name = klass.name.split(/::/).last
tag_name[0, 1] = tag_name[0, 1].downcase
- klass.instance_variable_set("@tag_name", tag_name)
- klass.instance_variable_set("@have_content", false)
+ klass.instance_variable_set(:@tag_name, tag_name)
+ klass.instance_variable_set(:@have_content, false)
end
def install_must_call_validator(prefix, uri)
View
8 lib/xmlrpc/parser.rb
@@ -650,10 +650,10 @@ def initialize
if defined? XML::DOM::Builder
return if defined? XML::DOM::Node::DOCUMENT # code below has been already executed
klass = XML::DOM::Node
- klass.const_set("DOCUMENT", klass::DOCUMENT_NODE)
- klass.const_set("TEXT", klass::TEXT_NODE)
- klass.const_set("COMMENT", klass::COMMENT_NODE)
- klass.const_set("ELEMENT", klass::ELEMENT_NODE)
+ klass.const_set(:DOCUMENT, klass::DOCUMENT_NODE)
+ klass.const_set(:TEXT, klass::TEXT_NODE)
+ klass.const_set(:COMMENT, klass::COMMENT_NODE)
+ klass.const_set(:ELEMENT, klass::ELEMENT_NODE)
end
end
Something went wrong with that request. Please try again.