Cleaning up the pre_args logic by krainboltgreene · Pull Request #112 · jruby/jruby

krainboltgreene · 2012-01-21T02:54:04Z

Reducing duplicate code
Clarifying variable names
Removing ternary block
Dropping for .. in block for scoped #each on enumerable

My "test case":

require 'benchmark'

class ArgsNode
  attr_accessor :pre
end

class Pre
  attr_accessor :child_nodes
end

class MultipleAsgn19Node
end

class MultipleAsgn18Node
end

class ANode < MultipleAsgn19Node
end

class BNode < MultipleAsgn18Node
  def name
    self.class.name
  end
end

pre = Pre.new
args_node = ArgsNode.new
nodes = [ANode.new, ANode.new, BNode.new] * 100
pre.child_nodes = nodes
args_node.pre = pre


report = Benchmark.bmbm do |x|
  x.report("old") { 1000.times {
    @args_ary = []
    required_pre = args_node.pre
    if required_pre
      for req_pre_arg in required_pre.child_nodes
        if req_pre_arg.kind_of? MultipleAsgn19Node
          @args_ary << [:req]
        else
          @args_ary << [:req, req_pre_arg ? req_pre_arg.name.intern : nil]
        end
      end
    end
  } }

  x.report("new") { 1000.times {
    @nargs_ary = []
    args_node.pre.child_nodes.each do |node|
      node_itern = node.name.intern unless node.kind_of? MultipleAsgn19Node
      @nargs_ary << [:req]
      @nargs_ary.last << node_itern if node_itern
    end if args_node.pre
  } }
end

puts "New is same as old? #{@args_ary == @nargs_ary}"
puts "New is #{report.map(&:to_s).map(&:split).map(&:last).map(&:to_f).inject(:/) * 100 - 100}% faster"

timfel · 2012-01-21T09:13:45Z

See my comment on #113
Von: Kurtis Rainbolt-Greene
Gesendet: 21.01.2012 03:54
An: Tim Felgentreff
Betreff: [jruby] Cleaning up the pre_args logic (#112)

Reducing duplicate code
- Clarifying variable names
- Removing ternary block
- Dropping for .. in block for scoped #each on enumerable

You can merge this Pull Request by running:

git pull https://github.com/krainboltgreene/jruby patch-1

Or you can view, comment on it, or merge it online at:

#112

-- Commit Summary --

Cleaning up the pre_args logic

-- File Changes --

M lib/ruby/shared/jruby/core_ext/method.rb (15)

-- Patch Links --

https://github.com/jruby/jruby/pull/112.patch
https://github.com/jruby/jruby/pull/112.diff

Reply to this email directly or view it on GitHub:
#112

krainboltgreene · 2012-01-21T18:43:54Z

I updated with a benchmark, and on MRI it comes out to 1% to -5% faster. :/ Installing JRuby to test speed on that impl.

timfel · 2012-01-22T16:41:48Z

I let your bench run 100,000 times instead of 1,000. On MRI HEAD it's between -3% and -6% for me, JRuby HEAD is around -15% to -20%.

krainboltgreene · 2012-01-22T18:08:03Z

Well shit, that blows. I think this is a general improvement on readability, but clearly not a performance increase. Thoughts?

On Jan 22, 2012, at 10:41 AM, Tim Felgentreffreply@reply.github.com wrote:

I let your bench run 100,000 times instead of 1,000. On MRI HEAD it's between -3% and -6% for me, JRuby HEAD is around -15% to -20%.

Reply to this email directly or view it on GitHub:
#112 (comment)

BanzaiMan · 2012-04-01T04:53:52Z

See #113. I'm closing this for now.

f06add1 Add a C-API "rb_hash_clear" 5224ff5 Add a String#split spec with 0 limit 73765b9 Little fixes for the new spec. f02b30b Added a few new specs for save/restore of $! a32e961 Spec for fnmatch with multibyte characters. See #3248. c567cba Add Module.const_get returning private constant test 2cf553f Socket::SO_REUSEADDR is non-zero/zero, update spec accordingly 83fa329 Fix spec to call Socket::Option#int. 82c8b9b Add spec for TCPServer having REUSEADDR set by default. 335915c Add spec for Array#uniq on BasicObject subclass. 10d5296 add failing spec for calling pack_sockaddr_in with a nil port 534643c add failing spec for to_h not duplicating 6ef3783 Add spec for File.open with a unicode path. 9b55ac0 Add another example for Array#push. 52d84d7 The rescue keyword without classes will not rescue Exception 59b4a52 Updating spec to test large values a420066 Guard Process.fork with a feature guard 7dc431a Disable OS X on Travis as it slows the build considerably 871319b Merge pull request #116 from benlovell/master 9836945 Specify `superclass` in respect to `prepend` bdee7d6 Merge pull request #115 from nobu/bug/hypot-nil ba78974 Adjust number of arguments to the arity 0cb8c3c Exclude ruby-head on OS X, it is slow and pretty old. a01554e Merge pull request #114 from JuanitoFatas/language/class-and-module-keywords 7a20841 Better class & module name to avoid conflicts 4962f6a Add specs for class keyword cf575ac Add specs for module keyword e27464c Merge pull request #111 from kachick/grep_v 05cd690 Merge pull request #112 from kachick/rb_define_class_under 0d978d8 Add "rb_define_class_under"(C-API) specs for errors 8d65c2f Merge pull request #113 from kachick/nameerror-receiver 5be05e2 Merge pull request #110 from kachick/numeric-sign 86c91a1 Write NameError#receiver specs 3543656 Write Enumerable#grep_v specs 45c334e Write Numeric#negative? specs 6e164b0 Write Numeric#positive? specs 90d7a39 Merge pull request #108 from kachick/deprecate_constant2 8511340 Add Module#deprecate_constant specs d82a978 Fix Module#deprecate_constant for private constants f9a50cf Merge pull request #107 from kachick/deprecate_constant 3e9505b Write Module#deprecate_constant specs 9139fd8 Use Float constants for speical floats. 174ec42 add some TODOs aaf1d53 Update TODOs, most of them have been addressed. 590439b Merge pull request #106 from kotp/master 2c3aa6f Removed double 'ell' for more distinct character 5650778 Fix indentation in Symbol#inspect. d05a223 Merge pull request #104 from askl56/master e32af12 Merge pull request #103 from maxfurman/array_any_spec b1cde68 Merge pull request #105 from yous/trailing-whitespace 03218f1 Remove trailing whitespace 4d75ec8 fixed hash syntax to 1.9 f91cebd Array#any? is false if no block is given and everything in the array is falsy ff5e8ac Spec for Array#any? f3c5de1 Merge pull request #102 from maxfurman/hash_any_spec cb77288 Spec for Hash#any? git-subtree-dir: spec/ruby git-subtree-split: f06add13184226600436afebe5d7e1b5ae172326

Cleaning up the pre_args logic

695f354

BanzaiMan mentioned this pull request Apr 1, 2012

Cleaning up optional args logic #113

Closed

BanzaiMan closed this Apr 1, 2012

alexfalkowski mentioned this pull request May 1, 2014

java.lang.VerifyError: Bad type on operand stack #1674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cleaning up the pre_args logic#112

Cleaning up the pre_args logic#112
krainboltgreene wants to merge 1 commit intojruby:masterfrom
krainboltgreene:patch-1

krainboltgreene commented Jan 21, 2012

Uh oh!

timfel commented Jan 21, 2012

Uh oh!

krainboltgreene commented Jan 21, 2012

Uh oh!

timfel commented Jan 22, 2012

Uh oh!

krainboltgreene commented Jan 22, 2012

Uh oh!

BanzaiMan commented Apr 1, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

krainboltgreene commented Jan 21, 2012

Uh oh!

timfel commented Jan 21, 2012

Uh oh!

krainboltgreene commented Jan 21, 2012

Uh oh!

timfel commented Jan 22, 2012

Uh oh!

krainboltgreene commented Jan 22, 2012

Uh oh!

BanzaiMan commented Apr 1, 2012

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants