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

Was the --indy cli option removed? (1.7.16.1) #2231

Closed
PragTob opened this Issue Nov 24, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@PragTob
Copy link

PragTob commented Nov 24, 2014

Hi everyone,

just wondering if I'm extra ordinarily stupid right now or if the --indy option doesn't work/is deprecated.

tobi@tobi-desktop ~ $ jruby -v
jruby 1.7.16.1 (1.9.3p392) 2014-10-28 4e93f31 on OpenJDK 64-Bit Server VM 1.7.0_65-b32 +jit [linux-amd64]
tobi@tobi-desktop ~ $ jruby -h
#snip
  --(no-)indy       use invokedynamic (Java 7+) to optimize (perf++, startup--)
# snip
tobi@tobi-desktop $ jruby --indy  bin/run.rb 
jruby: unknown option --indy
tobi@tobi-desktop $ jruby --no-indy bin/run.rb 
jruby: unknown option --no-indy
tobi@tobi-desktop $ jruby -Xcompile.invokedynamic=true  bin/run.rb
# works (I guess)

Cheers + thanks,
Tobi

@headius

This comment has been minimized.

Copy link
Member

headius commented Nov 24, 2014

Huh...I wonder if I never added it? I know we had discussed it, but I don't remember what we decided. Can you blame the -h change and try to sort it out?

@PragTob

This comment has been minimized.

Copy link
Author

PragTob commented Nov 24, 2014

@headius it seems you added it back in 309576b seems accidental, as the commit message doesn't mention it

@headius

This comment has been minimized.

Copy link
Member

headius commented Nov 24, 2014

Ok, then I guess we don't have that flag and should remove the doco :-)

We can debate separately whether we should add it. @enebo?

@PragTob

This comment has been minimized.

Copy link
Author

PragTob commented Nov 24, 2014

I actually kind of liked to have that flag available, if I may join in :) But nothing too important :)

@enebo

This comment has been minimized.

Copy link
Member

enebo commented Nov 25, 2014

I don't like it as a flag and prefer a property and ultimately it is just an aggregate flag for several individual properties. Invokedynamic is an implementation detail you can toggle on or off, but for it to be at the same level of an option I think it needs to:

  1. Be well known as a feature
  2. Have a reason to easily be able to toggle it on or off

For 1, there is a subset of people who know about it but ultimately I think our JIT should be deciding how to use invokedynamic. Had there been no problems with it from a warmup or deoptimization (now mostly fixed) position then we would not even be having this discussion (it would always be on).

For 2, we currently have a reason for letting people enable ofdisable aspects of indy but it is a tunable. I suspect as a tunable you might not have this as an all or nothing feature unless the app is small. You might elect to enable some of the invokedynamic options until you get the performance/warmup which works for you. If this is the case, then you will be mostly tweaking the various indy properties. Having a main turn it all on property is cool to me though (it is a good starting point for tuning).

So my bias is towards leaving it only as a property and hoping that JVM fixes the warmup issues we still have with invokedynamic. In the future, I am also hoping IR will inform important sites to use indy and backoff on less hot sites with non-indy. This does not really even fit with the idea of properties; but I think it gives an idea as to why I don't think it is an option. This idea of selectively using indy is also sort of a backup plan if indy never warms up fast enough for us. We can still use indy but not enough for it to destroy warmup time.

@headius

This comment has been minimized.

Copy link
Member

headius commented Dec 2, 2014

Yeah, I agree with Tom. Or rather, I disagree, but it's because I want --indy for my local benchmarking/testing purposes. That's not a good enough reason to pollute our toplevel flags with yet one more :-)

I will fix up the doco.

@headius headius closed this Dec 2, 2014

headius added a commit that referenced this issue Dec 2, 2014

headius added a commit that referenced this issue Dec 2, 2014

@enebo enebo added this to the JRuby 1.7.17 milestone Dec 8, 2014

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.