Nashorn support (Java 8)

[sfnelson]

Java 8 includes a new javascript engine, Nashorn that provides a massive performance improvement over Rhino. Nashorn's Java interop is sufficiently different from Rhino that the existing Rhino versions don't work in Nashorn, even in Nashorn's Rhino interop mode.

It would be good to have a separate build similar to less-rhino, e.g. less-nashorn that provides the bindings required with work in the nashorn engine. This would support lessc integration into Java-based build tools.

[lukeapage]

Would be good to add support in 2.0 - In that branch(2_0_0) I'm refactoring the environment abstraction to make things like this alot easier.
Would you be interested in helping?

[sfnelson]

Sure. I've developed a less plugin for the leiningen (clojure) build tool: lein-less, and I've made some minimal changes to the generated less-rhino-1.7.2.js artifact to support nashorn, it should be fairly straightforward to add support for both. How would you like to proceed?

[lukeapage]

okay so the current branch is here

https://github.com/less/less.js/tree/2_0_0

this my current work in progress pull request

#1902

You are free to make pull requests against the 2_0_0 branch.

My current thoughts were to generate a single javascript file as we do today, but using browserify (as it does for the browser version on that branch). We would "just" need an environment file

https://github.com/less/less.js/tree/2_0_0/lib/less/environment/rhino.js

following the pattern from node.js and browser.js which implemented all the functions.

I haven't thought about changes to the rhino lessc file - I haven't thought much about how to do that, so feel free if you have any changes to that to just commit to there..

[sfnelson]

I'm looking at upgrading my plugin to support less 2.0.0 using browserify while supporting both Rhino and Nashorn VMs. I'd like to contribute my changes, including some Java bootstrapping code, but I'm confused about the multiple build systems referenced. Is the gradle build still used? Is build.yml still doing anything useful? Is your release process documented somewhere?

[lukeapage]

Fantastic! Thanks.
I have no idea if gradle is used, i think so, the build.yml i kept around
just for rhino but no idea if its even used in the last rhino build.
@SomMeri might be able to help

I wonder whether it might work better as a seperate repo.. what do you
think? If you agree i could create a less.java and give you developer
rights on it.

[SomMeri]

@sfnelson It would be awesome if you could contribute. Yes, rhino tests are currently gradle based. How to run/build rhino them is described in pull request #1806 . Last time I tried it was with 1.7.5 (probably) - they passed back then.

I do not know whether there is a place with release process described.

If we would separate rhino/nashortn from main repository, is there a way to run tests on a it every time main repo is pushed or release it when main is released? I do not think rhino tests are now run from travis after each commit, but I think they should be.

[ktngo]

Where I can find info and instruction on how to use less.js using Java 8 Nashorn Javascript Engine? When will Nashorn officially supported by Less.js like Less support Rhino before?

[SomMeri]

@ktngo As far as I know, nobody tried to make less.js to run in Nashorn yet. We would be thankful for pull request if someone would made the work needed.

[jknack]

It works:

  assets.load('lib/less-env-2.5.1.js');

  assets.load('lib/less-2.5.1.js', function (contents) {
    // See: https://bugs.openjdk.java.net/browse/JDK-8030198
    return contents.replace('this._currentDepth === 0', 'this._currentDepth <= 0');
  });

  var less = createFromEnvironment(lessenv);

asset.load just read a file from classpath and load it via nashorn: load function.

The less-env file was created with:

browserify node_modules/less-node/environment.js --s lessenv -o less-env-2.5.1.js

less with:

browserify node_modules/less/lib/less/index.js --s createFromEnvironment -o less-2.5.1.js

There is a bug in Nashorn where finally blocks got executed twice: https://bugs.openjdk.java.net/browse/JDK-8030198 and because of this... less.js produces no output.

A hack or workaround is required and that's why I have to replace:

this._currentDepth === 0 with this._currentDepth <= 0

The fix for Nashorn is coming... but it will be great if less can change it too.. feel less release are more easy and frequently than a JVM release :S

[Robinyo]

@jknack

with less.js-2.6.1 (Elementary OS, JDK 1.8):

browserify lib/less-node/environment.js --s lessenv -o less-env-2.6.1.js
browserify lib/less-node/index.js --s createFromEnvironment -o less-node-2.6.1.js

and a Maven project using the lesscss-maven-plugin version 1.7.0.1.1 with the following configuration

<lessJs>${project.basedir}/src/main/webapp/skin/third-party/less.js-2.6.1/dist/less-node-2.6.1.js</lessJs>     

I get the following error:

Failed to execute goal org.lesscss:lesscss-maven-plugin:1.7.0.1.1:compile (core) on project platform-ui-skin:
Execution core of goal org.lesscss:lesscss-maven-plugin:1.7.0.1.1:compile failed: Failed to initialize LESS compiler.
Encountered code generation error while compiling script: generated bytecode for method exceeds 64K limit.

Note: less-rhino-1.7.5.js works for Bootstrap v2.3.2 I need to migrate to Bootstrap v3.3.6

Any suggestions?

Env:
java version "1.8.0_77"
Java(TM) SE Runtime Environment (build 1.8.0_77-b03)
Java HotSpot(TM) 64-Bit Server VM (build 25.77-b03, mixed mode)

[jknack]

Have to look again, but did you replace the expression: this._currentDepth === 0 with this._currentDepth <= 0?

Anyway, I replaced nashorn with j2v8 and the results are here: http://jooby.org/doc/assets/#available-processors

[Robinyo]

-> Have to look again, but did you replace the expression: this._currentDepth === 0 with this._currentDepth <= 0?

I have now :)

// if (this._currentDepth === 0 && this._onSequencerEmpty) {
if (this._currentDepth <= 0 && this._onSequencerEmpty) {
    this._onSequencerEmpty();
}

(there is only one occurrence) and it works, but not with .less files that have a lot of @import's

Let me do some more testing...

[Robinyo]

-> (there is only one occurrence) and it works, but not with .less files that have a lot of @import's

Actually this is NOT correct as I missed the less-node-2.6.1.js configuration in a few spots and these where the ones that built, that is the less-rhino-1.7.5.js/lessJs> ones :(

[artfiedler]

I don't know about the browserify workarounds etc mentioned here... but I put together a nashorn implementation. At this moment here are the stats... running this on windows 10, jdk1.8.0_102... I didn't test on Linux yet but it should be working. Is there still interest in this?

c:...\less.js-2.7.1\bin>jjs lessjjsc -- ..\benchmark\benchmark.less
Finished: 6185.745009ms
I don't have node installed so I don't know how fast this runs on node, can someone post some timings? I'm measuring from just before less.render and first line in .then()

c:...\less.js-2.7.1\test\nashorn>jjs index.js --
13 Failed, 147 passed
I believe some of the failed tests actually pass just the output doesn't match, and a few minor tweaks might make a few more tests run successfully. Right now I know svg image size and source maps aren't implemented so those tests fail

(I have not posted code or made a pull request yet, FYI)

[sfnelson]

I'm certainly interested. What sort of changes did you need to make?

[artfiedler]

Changes...

• Zero changes to the original lib/less, and test case files..
• Added a lib/less-nashorn dir
• Copied of bin/lessc as bin/lessjjsc, modifications were a quick require() impl and a few other tweaks, a large majority of the file could be reused between node and jjs
• Copied test/*.js to test/nashorn/ which I modified index.js and less-test.js... I did not modify copy-bom.js I disabled that because it required some additional implementation and I didn't look into what "bom" is so I skipped it figured I would see how it runs against the main tests which as of last night was "8 failed and 152 passed"

I'll probably get a few more of those tests cases working tonight and tidy up a bit then post the code. I'm also going to post a wrapper that can be called from java for a project I was going to post on my GitHub with some other tools.

When I initially looked at converting my 1.x less-rhino wrapper to an updated version of less and to use with nashorn I noticed the build for rhino appears rather dated and delayed me getting going because I was trying to use less the same way as before, with dist/less.js & less-rhinoc.js ... after a bit I figured out that there is no dist of the core lib/less only lib/browser so that makes a java wrapper more complicated.

[sfnelson]

Interesting. A long while back I got the lessc functionality basically running with the browserify approach, but I wasn't able to get exceptions/errors working sufficiently well that they were useful for users. How are your error messages looking? If you create a branch I'd be interested to try it out!

[artfiedler]

An error message like this?
c:...\less.js-2.7.1\bin>jjs lessjjsc -- ..\test\less\errors\color-func-invalid-color.less
ERROR: ArgumentError: error evaluating function color: argument must be a color keyword or 3/6 digit hex e.g. #FFF in c:...\less.js-2.7.1\bin..\test\less\errors\color-func-invalid-color.less on line 2, column 10:
1 .test {
2 color: color("NOT A COLOR");
3 }

[artfiedler]

@sfnelson Finally checked it in, give it a whirl when you get a chance... you know these US Elections always taking up time! I also was able to get rid of the test\nashorn* dir and just merge it with the test\index.js etc and unified the code mostly

Edit: I should mention in this thread that the commit contains many more working tests, I modified copyBom so all those tests work now also... the only tests that are not working are source maps as that is not implemented. 6 Failed, 311 passed

[artfiedler]

@sfnelson Any update on testing the feature out?

[sfnelson]

Hey @artfiedler it looks interesting, but I don't think it's likely to be accepted upstream because of all the isNashornMode checks. Still it's a good start, and good to have a proof of concept. If you want to get it merged I'd suggest looking at ways of using browserify/similar builds to replace the conditional tests.

[artfiedler]

Hrmmm, the 2 references to isNashornMode in bin/lessjjsc actually aren't even used, I had left those in there thinking of merging bin/lessc and bin/lessjjsc. The other 10 references to isNashornMode are specifically in the test case set and not linked to lessjjsc or lib/* at all

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.