Closed bodiam closed 5 years ago
Keep in mind that there are some performance trade-offs for making this change. Asciidoctor.js isn't nearly as fast as Asciidoctor Ruby. If it's worth the trade-off, what this entails is bundling the asciidoctorj-nashorn artifact (instead of the future asciidoctorj-ruby artifact) once the change is made.
Also keep in mind that asciidoctorj-nashorn will require the use of JDK 8 (because Nashorn really isn't usable until then).
Hi Dan, thanks for the feedback. This was just a sort of reminder and discussion point. Probably it's not a good idea to do, but I mostly wanted to see if the plugin could be a bit smaller. Maybe that's just not possible right now.
Hi Dan, after a bit of hacking, I created something which works on JDK 1.6, 1.7 and JDK 1.8. I ran into the same problems with the 'char' being a reserved keyword and replaced it also with something else. But then I found this thread: http://discuss.asciidoctor.org/Asciidoctor-js-and-Java-td1369.html
After reading it, I'm a bit confused :-) For the IntelliJ plugin, I actually don't see much reason not to use the AsciiDoctorJS version. Most (all?) the plugin is doing, or should do, is rendering the Asciidoc to HTML, which is what the Javascript is doing perfectly. It also means slimming down the plugin from 50mb to 500 kb, which is quite nice.
What kind of issues do you see with this approach (so, using AsciidocJS instead of AsciidoctorJ, and using Rhino on Java 6/7, and Nashhorn on Java 8?
(btw, I also did a small benchmark by rendering a semicomplex document (this one: http://wildfly-mgreau.rhcloud.com/ad-editor/), and on JDK 1.6 it took around 800 ms, on JDK 1.8 it took 1.5 seconds.
I'm not sure why it's so slow on JDK 1.8, I expected something completely different, but that's maybe something for later.
For the IntelliJ plugin, I actually don't see much reason not to use the AsciiDoctorJS version.
I agree that it's a viable approach. There are, however, some limitations with doing this.
There might be some other related challenges for using Asciidoctor.js in the preview pane (I'll post them as I think of them). No doubt, you are trading one type of pain (the size of JRuby) with another type of pain (putting a lot more responsibility on the rendering environment).
There's no doubt that we can get the size of the plugin down to ~25MB using JRuby. There's no reason AsciidoctorJ has to be so large...we just haven't optimized it. JRuby 9000 (the next version) is going to shrink a bit. We might also be able to build a custom jruby-complete jar that cuts out some of the stuff we don't need. Even with a ~25MB download, you only have to download it once, then the author gets a really good experience after that.
I'm not sure why it's so slow on JDK 1.8
It's using a different JavaScript implementation (Nashorn instead of Rhino). While Nashorn is a much more compliant and complete JavaScript implementation than Rhino, it's also a lot slower right now. That should improve with JDK9.
I should probably also add that Asciidoctor.js isn't stable yet in Nashorn, so it might be too soon to rely on that combination IMO.
Keep in mind that there will be two different ways to use Asciidoctor.js in the JVM. AsciidoctorJ 1.5.5 will support running Asciidoctor.js "server-side" (as an alternate implementation under the covers). The other way is having the embedded browser run Asciidoctor.js like the Atom, Brackets and Chrome extensions do. We have to be clear how we are running Asciidoctor.js when we talk about it.
When we talk about the web view, it's important to realize that JDK6/7 uses a home-grown HTML4 embedded browser that uses Rhino, whereas JDK8 offers an embedded version of WebKit. If we toss Asciidoctor.js at WebKit, great things happen. If we toss Asciidoctor.js at that home-grown browser in JDK6/7, we could get weird and broken behavior.
Btw, you can use asciidoctor-all.js (or opal-0.6.2.js + asciidoctor-core.js) from the Asciidoctor.js repository (see https://github.com/asciidoctor/asciidoctor.js/tree/master/dist). However, to run it on JDK6, you have to change one line in Opal. It's the following line from asciidoctor-all.js:
https://github.com/asciidoctor/asciidoctor.js/blob/master/dist/asciidoctor-all.js#L3009
You need to add a backslash before the second forward-slash (the forward-slash inside the regexp). Otherwise, Rhino chokes. I've filed an upstream issue for this in Opal. See https://github.com/opal/opal/issues/651
Starting with #220 we experimentally support Ruby Asciidoctor extensions in this plugin. Changing to Asciidoctor.js conflicts with this task. I close it for now.
I tend to agree that sticking with AsciidoctorJ is the right direction for this plugin. Given that the plugin is in Java, it makes sense to use a Java library.
Currently, JRuby is bundled with the plugin. JRuby takes 23 MB, and replacing this with a more light version would slim down the plugin enormously.
See this: https://github.com/asciidoctor/asciidoctorj/issues/189