Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short-circuit chained lazy calls once a nil is encountered. #5307

Merged
merged 2 commits into from Sep 4, 2018

Conversation

Projects
None yet
1 participant
@headius
Copy link
Member

headius commented Sep 4, 2018

The original logic would continue to check nil for each lazy call,
even after a nil resulted from one of the elements. This new logic
will always branch to the end of the chain if a nil is encountered
anywhere along the way.

This combined with the indy BNil patch produces the best perf for
a benchmark of a long chain of calls against an always-nil value.

headius added some commits Sep 4, 2018

Short-circuit chained lazy calls once a nil is encountered.
The original logic would continue to check nil for each lazy call,
even after a nil resulted from one of the elements. This new logic
will always branch to the end of the chain if a nil is encountered
anywhere along the way.

This combined with the indy BNil patch produces the best perf for
a benchmark of a long chain of calls against an always-nil value.
@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 4, 2018

Ok, so benchmarks.

There's two versions of bench here; one has a chain that terminates immediately (first receiver is already nil. The second one terminates after the first call (first receiver is not nil but returns nil).

class X
  def y
    nil
  end
end;

def foo(x)
  x&.y&.y&.y&.y&.y&.y&.y&.y&.y&.y
end

loop {
  t = Time.now
  i = 0
  x = X.new # replace with x = nil to test immediate short-circuit
  while i < 100_000_000
    foo(x)
    i += 1
  end
  puts Time.now - t
}

Performance on JDK8 before and after, with and without indy:

BEFORE
[] ~/projects/jruby $ jruby blah4.rb
2.818749
2.918342
2.7041440000000003
2.89792
2.7052840000000002
2.739289
2.7195780000000003
2.6913270000000002
2.691073
2.677393
^C
[] ~/projects/jruby $ jruby -Xcompile.invokedynamic blah4.rb
1.428776
1.327828
1.081034
0.911455
0.9556359999999999
0.9412269999999999
0.9103530000000001
0.909707
0.9276660000000001
0.9147430000000001

AFTER
[] ~/projects/jruby $ jruby blah4.rb
2.813434
2.789313
2.63021
2.660738
2.597217
2.587168
2.598025
2.5840140000000003
2.592276
^C
[] ~/projects/jruby $ jruby -Xcompile.invokedynamic blah4.rb
1.1255909999999998
1.026116
0.8122520000000001
0.556724
0.5597460000000001
0.5557070000000001
0.559119
0.556427
0.559196
0.5545410000000001
0.550336

Oddly enough something seems to have broken performance on GraalVM EE for this case:

GraalVM EE rc6 AFTER

[] ~/projects/jruby $ jruby blah4.rb
4.0804920000000005
3.1149430000000002
2.852968
2.833921
2.838522
2.840038
2.822295
2.846982
2.8676719999999998
2.856983
^C
[] ~/projects/jruby $ jruby -Xcompile.invokedynamic blah4.rb
46.42731
34.626850000000005
33.708152000000005
33.932513
33.658346
^C

If I modify the bench to short-circuit immediately (first receiver is nil), GraalVM does much better:

[] ~/projects/jruby $ jruby -Xcompile.invokedynamic blah4.rb
1.353125
0.62861
0.57512
0.5497620000000001
0.656639
0.5461330000000001
0.560104
0.547803
0.5502950000000001
0.514778
0.49834
0.495632
0.488224

I'm unsure why indy degrades the not-quite-immediate-short-circuit case so severely on GraalVM.

Graal JIT in a recent JDK 11 build also degrades the same way.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 4, 2018

Ok, the indy + graal performance degradation appears to be something unrelated to BNil and lazy operator work from today.

@headius headius merged commit 44c6f42 into jruby:master Sep 4, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@headius headius added this to the JRuby 9.2.1.0 milestone Sep 4, 2018

@headius headius deleted the headius:chained_lazy branch Sep 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.