Skip to content

Provide a collector for indy varargs boxing#6630

Merged
headius merged 3 commits intojruby:masterfrom
headius:collect_better
Mar 26, 2021
Merged

Provide a collector for indy varargs boxing#6630
headius merged 3 commits intojruby:masterfrom
headius:collect_better

Conversation

@headius
Copy link
Member

@headius headius commented Mar 26, 2021

It turns out that MethodHandle.asCollector is very slow for
subclasses of Object[] due to double-allocation and double-copying
done by the OpenJDK logic. To avoid this overhead, we use 10-wide
overloads of our own collector utility functions and pass to
Binder.collect, which now supports the
MethodHandles.collectArguments decorator.

Fixes #6628

@headius
Copy link
Member Author

headius commented Mar 26, 2021

Performance of modified dig benchmark from #6628 (Java 11, +ParallelGC):

require 'benchmark/ips'

puts RUBY_DESCRIPTION

array = [[[[[[[[[[[14]]]]]]]]]]]

Benchmark.ips do |x|
  x.report('dig-01') {|i| while i>0; i-=1; array.dig(0); end }
  x.report('dig-02') {|i| while i>0; i-=1; array.dig(0, 0); end }
  x.report('dig-03') {|i| while i>0; i-=1; array.dig(0, 0, 0); end }
  x.report('dig-04') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0); end }
  x.report('dig-05') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0); end } # easy to find examples this long
  x.report('dig-06') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0, 0); end }
  x.report('dig-07') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0, 0, 0); end }
  x.report('dig-08') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0, 0, 0, 0); end } # possible to find examples this long
  x.report('dig-09') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0, 0, 0, 0, 0); end }
  x.report('dig-10') {|i| while i>0; i-=1; array.dig(0, 0, 0, 0, 0, 0, 0, 0, 0, 0); end }
  x.compare!
end

No indy:

              dig-01     38.572M (± 4.8%) i/s -    195.184M in   5.072156s
              dig-02     40.883M (± 9.2%) i/s -    202.908M in   5.019322s
              dig-03     29.216M (± 6.0%) i/s -    147.446M in   5.067869s
              dig-04     24.229M (±14.5%) i/s -    116.700M in   5.008082s
              dig-05     21.963M (± 3.0%) i/s -    110.112M in   5.018112s
              dig-06     19.876M (± 3.9%) i/s -     99.521M in   5.014908s
              dig-07     17.807M (± 6.0%) i/s -     90.336M in   5.096993s
              dig-08     16.079M (± 5.5%) i/s -     80.465M in   5.021685s
              dig-09     14.032M (± 6.0%) i/s -     70.755M in   5.062774s
              dig-10     13.674M (± 4.9%) i/s -     69.170M in   5.070931s

With indy, before:

              dig-01     22.006M (± 7.5%) i/s -    109.948M in   5.026393s
              dig-02     20.545M (± 5.0%) i/s -    104.253M in   5.087895s
              dig-03     17.492M (± 7.7%) i/s -     88.326M in   5.083337s
              dig-04     15.416M (± 8.5%) i/s -     76.567M in   5.009796s
              dig-05     10.026M (±17.8%) i/s -     49.617M in   5.112008s
              dig-06      6.033M (±28.6%) i/s -     28.628M in   5.207503s
              dig-07      8.400M (±22.2%) i/s -     40.710M in   5.145922s
              dig-08      6.176M (±23.2%) i/s -     29.795M in   5.026196s
              dig-09      5.184M (±10.3%) i/s -     26.370M in   5.141858s
              dig-10      4.955M (± 7.4%) i/s -     24.656M in   5.004148s

With indy, after:

              dig-01     90.549M (± 9.3%) i/s -    447.936M in   4.994618s
              dig-02     69.627M (± 8.7%) i/s -    349.313M in   5.058336s
              dig-03     46.386M (± 7.5%) i/s -    233.228M in   5.059314s
              dig-04     38.318M (± 7.3%) i/s -    190.748M in   5.005450s
              dig-05     29.669M (±10.2%) i/s -    147.699M in   5.045310s
              dig-06     26.354M (± 7.2%) i/s -    133.189M in   5.082447s
              dig-07     23.083M (± 4.1%) i/s -    116.419M in   5.052934s
              dig-08     19.891M (± 5.0%) i/s -    100.399M in   5.060131s
              dig-09     17.120M (± 8.4%) i/s -     86.138M in   5.071891s
              dig-10     16.170M (± 8.3%) i/s -     80.678M in   5.030556s

@headius
Copy link
Member Author

headius commented Mar 26, 2021

Ran into an issue running optcarrot:

$ jruby -J-XX:+UnlockExperimentalVMOptions -Xcompile.invokedynamic -J-XX:+UseParallelGC -X+C -Xjit.threshold=0 -Xfixnum.cache=false bin/optcarrot --benchmark examples/Lan_Master.nes
java.lang.IllegalArgumentException: no argument type to append
	at java.lang.invoke.MethodHandleStatics.newIllegalArgumentException(MethodHandleStatics.java:139)
	at java.lang.invoke.MethodHandles.insertArgumentsChecks(MethodHandles.java:2406)
	at java.lang.invoke.MethodHandles.insertArguments(MethodHandles.java:2369)
	at com.headius.invokebinder.transform.Insert.up(Insert.java:99)
	at com.headius.invokebinder.Binder.invoke(Binder.java:1171)
	at com.headius.invokebinder.Binder.invokeVirtual(Binder.java:1283)
	at com.headius.invokebinder.Binder.invokeVirtualQuiet(Binder.java:1304)
	at com.headius.invokebinder.SmartBinder.invokeVirtualQuiet(SmartBinder.java:1026)
	at org.jruby.ir.targets.Bootstrap.buildMethodMissingHandle(Bootstrap.java:666)
	at org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:175)
	at Users.headius.projects.optcarrot.lib.optcarrot.nes.RUBY$method$reset$4(/Users/headius/projects/optcarrot/lib/optcarrot/nes.rb:35)
	at Users.headius.projects.optcarrot.lib.optcarrot.nes.RUBY$method$reset$4$__VARARGS__(/Users/headius/projects/optcarrot/lib/optcarrot/nes.rb)
	at org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:80)
	at org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:207)
	at Users.headius.projects.optcarrot.lib.optcarrot.nes.RUBY$method$run$8(/Users/headius/projects/optcarrot/lib/optcarrot/nes.rb:75)
	at Users.headius.projects.optcarrot.lib.optcarrot.nes.RUBY$method$run$8$__VARARGS__(/Users/headius/projects/optcarrot/lib/optcarrot/nes.rb)
	at org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:80)
	at org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:207)
	at bin.optcarrot.RUBY$script(bin/optcarrot:6)
	at java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:627)
	at org.jruby.ir.Compiler$1.load(Compiler.java:89)
	at org.jruby.Ruby.runScript(Ruby.java:1205)
	at org.jruby.Ruby.runNormally(Ruby.java:1128)
	at org.jruby.Ruby.runNormally(Ruby.java:1146)
	at org.jruby.Ruby.runFromMain(Ruby.java:958)
	at org.jruby.Main.doRunFromMain(Main.java:400)
	at org.jruby.Main.internalRun(Main.java:292)
	at org.jruby.Main.run(Main.java:234)
	at org.jruby.Main.main(Main.java:206)

@headius headius marked this pull request as draft March 26, 2021 09:04
@headius
Copy link
Member Author

headius commented Mar 26, 2021

The issue in optcarrot was due to indy binding method_missing with the wrong number of arguments after my change. The arity of the method_missing call and the arity of the collector did not match.

I added a spec for a zero-arity method_missing call which will catch this issue in the future.

This is ready (again).

@headius headius marked this pull request as ready for review March 26, 2021 18:31
@headius headius changed the base branch from jruby-9.2 to master March 26, 2021 21:28
headius added 3 commits March 26, 2021 16:29
It turns out that MethodHandle.asCollector is very slow for
subclasses of Object[] due to double-allocation and double-copying
done by the OpenJDK logic. To avoid this overhead, we use 10-wide
overloads of our own collector utility functions and pass to
Binder.collect, which now supports the
MethodHandles.collectArguments decorator.

Fixes jruby#6628
Would have caught the method_missing glitch
@headius headius modified the milestones: JRuby 9.2.17.0, JRuby 9.3.0.0 Mar 26, 2021
@headius headius merged commit 8c49344 into jruby:master Mar 26, 2021
@headius headius deleted the collect_better branch March 26, 2021 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#dig slower with invokedynamic than without

1 participant