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

Use nashorn in java 8 instead of clj-v8? #14

Open
magnars opened this issue Mar 19, 2014 · 19 comments
Open

Use nashorn in java 8 instead of clj-v8? #14

magnars opened this issue Mar 19, 2014 · 19 comments

Comments

@magnars
Copy link
Owner

magnars commented Mar 19, 2014

Would be nice to avoid the trouble with binaries, and also get support on windows.

@radhikalism
Copy link
Collaborator

+1

I'd intended to raise this issue too. Started tinkering with it today, looks promising.

Could be nice to allow some pluggable choice of engines, including V8, Nashorn, and perhaps in the future a *monkey or other JS implementation.

@magnars
Copy link
Owner Author

magnars commented Mar 19, 2014

Started tinkering with it today, looks promising.

Excellent! Do let me know what you find.

@radhikalism
Copy link
Collaborator

So it looks this approach mostly works:

  • Add a JSR-223 API implementation in clj-v8, making it a discoverable ScriptEngine service
  • Refactor minify.clj in Optimus to use only javax.script.* interfaces, decoupling it from V8
  • Add an environ-based config setting to prefer JS engines in some order

Result: an Optimus user can declare which JS engines they prefer (v8, nashorn, rhino), as late as boot-time, and the appropriate JS engine is loaded by javax.script.ScriptEngineManager if available.

Have a look at radhikalism/optimus@650777e and radhikalism/clj-v8@5fc6dd1

I'll PR clj-v8 with the JSR-223 API implementation first, after some minor fixes. (Will also need to get my head around clj-v8's build process for multiple platforms, as this involves a minor change to its native v8wrapper.)

Meanwhile, under Nashorn, one of Optimus' CSS minification tests fails with a minor difference. Interestingly, everything passes under Rhino.

@cjohansen
Copy link
Collaborator

Minor differences in the minified output doesn't make much of a difference IMO.

@magnars
Copy link
Owner Author

magnars commented Mar 21, 2014

Wow, this is some great work. Thank you!

I wonder how Nashorn is able to present a runtime for CSSO such that it still works, but produces a slightly different output from the other JS engines. That is pretty weird, and a little disconcerting.

@magnars
Copy link
Owner Author

magnars commented Oct 21, 2014

Any luck with the clj-v8 pull request?

@radhikalism
Copy link
Collaborator

Sorry, I didn't get any further than some OS compatibility fixes (circleci/clj-v8#11 — still open and waiting), which was a prerequisite for my deployment circumstances at the time. I'll try separating those fixes from a JSR-223 patch and see if the latter gets any traction.

clj-v8 is maintained but seems to move slowly. Pragmatically, I'm tempted to suggest two separate code paths for minification in Optimus, one for clj-v8 and one for all JSR-223 backends, with a view to folding clj-v8's code path into the latter if and when it supports JSR-223 too.

@magnars
Copy link
Owner Author

magnars commented Oct 21, 2014

Thanks for the update! It is my impression too that clj-v8 is slow moving.

How about we move away from clj-v8 entirely in a 1.0 release, moving over to Nashorn and Java 8. We could create a tag for people who cannot move to Java 8. Any thoughts on that?

@radhikalism
Copy link
Collaborator

I like the idea of making Nashorn and Java 8 the default for 1.0.

For new Optimus users with earlier Java versions, Rhino is not entirely terrible, and DynJS could be an option.

But continuing support for clj-v8 would be nice for existing users. Perhaps a third-party JSR-223 adapter would work. It makes more sense to patch clj-v8 directly (as in my fork), but I think those classes could be factored out into another project that depends on clj-v8, for Optimus to use.

@magnars
Copy link
Owner Author

magnars commented Oct 22, 2014

Would a third-party JSR-223 adapter work? I thought it was part of Java 8.

On Tue, Oct 21, 2014 at 3:30 PM, Abhishek Reddy notifications@github.com
wrote:

I like the idea of making Nashorn and Java 8 the default for 1.0.

For new Optimus users with earlier Java versions, Rhino is not entirely
terrible, and DynJS could be an option.

But continuing support for clj-v8 would be nice for existing users.
Perhaps a third-party JSR-223 adapter would work. It makes more sense to
patch clj-v8 directly (as in my fork), but I think those classes could be
factored out into another project that depends on clj-v8, for Optimus to
use.


Reply to this email directly or view it on GitHub
#14 (comment).

@radhikalism
Copy link
Collaborator

JSR-223 javax.script APIs are in Java 7 too (hence the Rhino scripting engine). Only the Nashorn scripting engine is in Java 8.

What I mean is that a hypothetical "clj-v8-jsr223" project could provide an implementation of JSR-223 (as in my fork) merely hooking into clj-v8 proper as a dependency. Optimus users could then optionally depend on "clj-v8-jsr223", overriding the defaults of Nashorn (on 8) or Rhino (on 7). Maybe.

@magnars
Copy link
Owner Author

magnars commented Oct 22, 2014

I see, how nice. That sounds like the best path forward, if we can't get your patch into clj-v8. Is it something you have got time for and would be interested in doing at all?

@radhikalism
Copy link
Collaborator

Update since this has come up again and I had some time to spare:

I've split the JSR-223 changes out of my clj-v8 fork, into clj-jsr223-v8. https://github.com/arbscht/clj-jsr223-v8/

This new project will be released independently but can loosely track clj-v8 releases (should any ever appear). It adapts clj-v8's API into a discoverable/pluggable JSR-223 form. So I expect Optimus can depend on clj-jsr223-v8 instead -- that is, after appropriate refactoring to use javax.script.

Once Optimus uses javax.script, the choice of scripting engine can be made configurable as a new feature, and we can play with Nashorn etc.

To recap: my next steps are to release clj-jsr223-v8; then try to assemble a pull request with updated script engine invocation in Optimus (replacing the direct dependency on clj-v8); and eventually make the script engines configurable.

How does that sound?

@magnars
Copy link
Owner Author

magnars commented Oct 16, 2015

That sounds brilliant!
fre. 16. okt. 2015 kl. 07.34 skrev Abhishek Reddy <notifications@github.com

:

Update since this has come up again and I had some time to spare:

I've split the JSR-223 changes out of my clj-v8 fork, into clj-jsr223-v8.
https://github.com/arbscht/clj-jsr223-v8/

This new project will be released independently but can loosely track
clj-v8 releases (should any ever appear). It adapts clj-v8's API into a
discoverable/pluggable JSR-223 form. So I expect Optimus can depend on
clj-jsr223-v8 instead -- that is, after appropriate refactoring to use
javax.script.

Once Optimus uses javax.script, the choice of scripting engine can be
made configurable as a new feature, and we can play with Nashorn etc.

To recap: my next steps are to release clj-jsr223-v8; then try to assemble
a pull request with updated script engine invocation in Optimus (replacing
the direct dependency on clj-v8); and eventually make the script engines
configurable.

How does that sound?


Reply to this email directly or view it on GitHub
#14 (comment).

@radhikalism
Copy link
Collaborator

Some recent news on this:

  • I've updated clj-jsr223-v8, a wrapper around clj-v8, that extends the pluggable JSR-223 API.
  • There are releases of clj-jsr223-v8 on Clojars
  • I have a recent fork of Optimus that depends on clj-jsr223-v8 via javax.script.* instead of directly on clj-v8, and it seems to work.
    • This Optimus can be configured using environ to switch between backend engines with a simple switch, e.g. OPTIMUS_JS_ENGINE=nashorn lein repl will give you a REPL where Optimus will use Nashorn on JDK8 (among other ways of using environ).
    • All Optimus tests pass for me.

I think it's about ready for a PR. What do you think @magnars?

@magnars
Copy link
Owner Author

magnars commented May 1, 2016

Oh, excellent. This looks great. That PR would be very welcome indeed. :)

@atroche
Copy link

atroche commented Sep 13, 2016

wow, looking forward to this

@piotr-yuxuan
Copy link

Any news about this PR?

@magnars
Copy link
Owner Author

magnars commented Apr 9, 2018

That's a good question. You can see the story of the PR here: #52. @arbscht was the primus motor for this fix, and it seems he may have run out of steam - it turned out to be quite the task. If someone who uses Windows wants to figure out what remains to be done and take over, I'm all for it.

We need to use JSR-223 instead of just defaulting to Nashorn always, because Nashorn turned out to be an order of magnitude slower than v8 when I did my testing. Minifying bootstrap.css with clean-css on Nashorn took ~10 seconds. That's a couple years ago now, so things are hopefully better now.

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

5 participants