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

New release #47

Closed
gsamokovarov opened this issue Jun 8, 2014 · 28 comments
Closed

New release #47

gsamokovarov opened this issue Jun 8, 2014 · 28 comments

Comments

@gsamokovarov
Copy link

Hi there,

Is there a chance to release a new version with the JRuby interpreted mode support baked in? Its been sitting in master for a while now.

As part of Rais 2014 GSoC, we're doing a debug console and we really wanna support JRuby as a lot of people are running it in production, hence in development too. Even with preconditions (interpreter mode being opt-in), we think the JRuby world will benefit of that.

@banister
Copy link
Owner

banister commented Jun 9, 2014

I'll try for next weekend, i've been pretty busy recently sorry. :)

@gsamokovarov
Copy link
Author

Thanks @banister :)

@gsamokovarov
Copy link
Author

Hey @banister, can I ping you again :)

@Ch4s3
Copy link

Ch4s3 commented Jul 16, 2014

Yeah, I would love this to be a feature.

@guilleiguaran
Copy link

+1 for this!!

@badosu
Copy link
Collaborator

badosu commented Aug 5, 2014

@guilleiguaran @gsamokovarov @Ch4s3

I added this experimental support thanks to the help of @headius, which created most of the code, I just adapted it to the b_o_c api.

Since then, we changed the specs trying to make the api consistent and stable between ruby implementations.

I feel like this is not really something mature in the current state, but I would be glad if we could release this experimental support, gather feedback and improve it.

It's possible even that JRuby has more introspection capabilities, on the state of the art, that we can use now, compared to the time this was created.

Please tell me if you used it, how it worked or crashed, and let me report this to @banister so we can get a new release.

Also, at least it won't break on installs.

@gsamokovarov
Copy link
Author

Hey @badosu,

Thanks for the response! If not a stable release, can we try a beta release? I'm asking for that as the first Rails 4.2 beta is going to be released soon and our gem that's in the default application Gemfile won't even bundle on JRuby in 0.7.2.

We can't get around it as we can't depend on a git version in our gemspec. The alternative is to let users install binding_of_caller manually, but I'll hate to complicate the beta install even more.

Nobody expects stability out of a beta release and it won't be picked up in the more general catch all gem 'binding_of_caller', so the majority of the existing installs won't break even on bundle update. But projects, like web-console will benefit of being able to test the unstable binding_of_caller during the Rails 4.2 beta.

@badosu, @banister What do you guys think?

@banister
Copy link
Owner

@badosu Do you want to be responsible for this? Give me your email address and i'll give you gem push access to binding_of_caller

@ryandao
Copy link

ryandao commented Aug 18, 2014

👍 for this. I'll love to at least be able to bundle install on JRuby so we can add binding_of_caller as a dependency.

@badosu
Copy link
Collaborator

badosu commented Aug 18, 2014

So, this is the commit that allows installing b_o_c under jruby and rbx: 266db4c

Let's release it!

@guilleiguaran
Copy link

:shipit:

@headius
Copy link

headius commented Aug 18, 2014

I want to remind everyone again that this binding_of_caller patch for JRuby only works if you turn off the JIT (the new --dev flag does that too).

At the moment, it will not work at all on JRuby 9k -- a completely different hack will be required because of the new runtime.

@badosu
Copy link
Collaborator

badosu commented Aug 18, 2014

@headius

From what I see, the most important right now is avoiding C-exts compilation under Jruby/RBX, that is crashing bundles that can't control whether b_o_c is included or not.

Do you have any idea how we can implement b_o_c functionality under JRuby 9K, any new API?

@headius
Copy link

headius commented Aug 18, 2014

binding_of_caller under JRuby 9k may be a challenge. The interpreter no longer uses the same externally-accessible structures to hold local variables. This is the real problem with binding_of_caller...it requires a lowest-common-denominator implementation that doesn't optimize method call state at all, as in MRI and Rubinius. Eventually both of those implementations will want to do more optimization.

@subbuss and @tom_enebo might have thoughts on binding_of_caller, since they implemented the new runtime.

@subbuss
Copy link

subbuss commented Aug 19, 2014

In the new 9k runtime, arbitrary read/write access to bindings up the call stack is not possible in the default production-use scenario because, as @headius said, the variables might have been allocated to JVM locals rather than in a heap structure.

But, if this is for debugging reasons only, we could conceivably enable this mode with a special command-line flag to JRuby. This should be simple since we already have checks for when a binding cannot be flattened into locals and requires a heap presence => we just need to add an additional boolean test there, and this flag should work independent of whether the runtime is in interpreter or JIT mode.

@gsamokovarov
Copy link
Author

Thanks @subbuss for chiming in! Yeah, we only need this for debugging purposes, so the flag idea can work great for our use case.

@badosu
Copy link
Collaborator

badosu commented Aug 19, 2014

@subbuss Users already bother to configure their application to run on interpreted mode for development, so using a different flag would probably not be an issue. Thanks for the helpful input!

@enebo
Copy link

enebo commented Aug 19, 2014

@headius If the current code requires DynamicScope to exist for each frame then that gem will not require any changes if we do as @subbuss suggests and use CLI flag to specify we need to support this. In that case we either toggle all IRScopes to have the flag BINDING_HAS_ESCAPED (frontrunner in my mind) or REQUIRES_DYNSCOPE. Our only difficulty is doing that in the cleanest way possible.

I am adding this comment mostly to say this...At some point we want to decouple dynamicscope from staticscope so we don't have to access dynamicscope through a staticscope. If we do this then we must make a new API which 1.7.x and 9k both have. This will affect binding_of_caller if we do so I am throwing that out there. fwiw, we may not have time to make this change since 9k has been dragging on so long.

@enebo
Copy link

enebo commented Aug 19, 2014

Ah I just looked at your impl. I believe my worry about changing the impl for 9k will be simple enough since we can just add some version check and have a small amount of extra logic for new design and the current design for accessing scopes in your source. We just need to make sure we help with that when/if we make the change I was alluding to in my last comment.

@badosu
Copy link
Collaborator

badosu commented Aug 19, 2014

@gsamokovarov I just don't understand how a Rails upgrade would crash the bundle due to b_o_c, could you describe this?

@enebo
Copy link

enebo commented Aug 19, 2014

One more note for 9k support since it popped into my head. If 9k JIT knows binding has escaped it probably will need to use same heap storage for vars meaning that in 9k that JIT can work the b_o_c. It be able to optimize local variable access but it will probably run a heck of a lot faster.

@gsamokovarov
Copy link
Author

@badosu In Rails 4.2, for every newly generated Rails application, we carry a new default gem -- web-console. It depends on binding_of_caller in its gemspec, which means bundle will pick the latest stable version 0.7.2 for the application Gemfile.lock. This makes bundle install to fail on JRuby.

I can work around that by adding the master binding_of_caller in the default Gemfile, if it was generated on JRuby or document you need it for JRuby in the guides, but I would like to avoid that.

If there is a beta release, I can depend on it in web-console gemspec and I won't have to confuse the users even more. I can't depend on a git/commit in the gemspec file, because of how rubygems work.

@badosu
Copy link
Collaborator

badosu commented Aug 19, 2014

@gsamokovarov Great! Thanks for the context.

@banister
Copy link
Owner

@gsamokovarov i just saw a demo of webconsole --- just curious, why did you rewrite the thing for the error page when there's already better_errors which does exactly the same thing ? :) (no offense -- just curious)

@Ch4s3
Copy link

Ch4s3 commented Aug 20, 2014

The 9k issue sounds interesting. I'd love to jump in and help if I can.

@badosu
Copy link
Collaborator

badosu commented Aug 20, 2014

@gsamokovarov Can you test webconsole against https://github.com/banister/binding_of_caller/releases/tag/v0.7.3.pre1?

I will push this to rubygems at the end of the day, but I want to identify any issues before that.

@gsamokovarov
Copy link
Author

@badosu I tried it out on a dummy testing app, seems to work with the interpreted mode :)

@badosu
Copy link
Collaborator

badosu commented Aug 20, 2014

@gsamokovarov http://rubygems.org/gems/binding_of_caller/versions/0.7.3.pre1

I am closing this issue for now, if there are any issues again please report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants