beanshell / beanshell

Beanshell scripting language
Apache License 2.0
815 stars 183 forks source link

Compiled BeanShell more strict with semi colons #529

Closed davidekholm closed 4 years ago

davidekholm commented 5 years ago

BeanShell's engine (bsh.engine.BshScriptEngine) reports to implement Compileable, still, compiled code is strict about single statements requiring semicolons. This makes it hard to transition from interpreted to compiled code without forcing 3:rd party script developers to append semicolons.

Please allow single statements to not have to be semicolon separated, even for compiled BeanShell code.

nickl- commented 5 years ago

@davidekholm I am not sure I follow what you are trying to suggest.

Could you perhaps give a code example comparing how it works currently and how you would prefer it work?

The semicolon is a java language syntax (also javascript) which indicates the end of statement because the interpreter does not read whitespace (you can type many statements on the same line or use multiple lines for a single statement).

The bsh shell allows statements without semicolon by adding an empty statement ; when empty line is submitted, this is a valid java statement which conveniently ends the unterminated statement.

How would you suggest we detect end of statement without a semicolon?

davidekholm commented 5 years ago

It's easy. Interpreted BeanShell allows single statements to skip the trailing semicolon, but compiled BeanShell doesn't. This makes it hard to transition scriptlets from interpreted to compiled form. I simply wish the same relaxed syntax for single statements to be applied to both compiled and interpreted BeanShell code.

Example:

import javax.script.*; mgr = new ScriptEngineManager(); se = mgr.getEngineByExtension("bsh"); se.eval("3+5"); // Works se.compile("3+5"); // Throws javax.script.ScriptException: Parse error at line 2, column 1. Encountered: }

nickl- commented 5 years ago

Understood, thank you for the explanation.

davidekholm commented 5 years ago

You're welcome. The jAlbum ecosystem has over 160 plugins ("skins") that use BeanShell scripting. A ton of those scriptlets use the relaxed syntax. In the old days of JSP scriptlets (which jAlbum mimics) one was also allowed to skip the trailing semicolon for single statements, for instance scriptlets like these: <%= 3+5 %>

nickl- commented 4 years ago

The compile functionally is intended for use as complete scripts, not scriptlets. If the script to be compiled is not valid and contains parser errors, like a missing line terminator ;, it will fail.

Closed: works as implemented

davidekholm commented 4 years ago

The ONLY relaxation I asked for was the same relaxation that holds true for interpreted BeanShell and that is that single statement scriptlets may have the semicolon omitted.

nickl- commented 4 years ago

@davidekholm I understand what you asked for but you are not hearing me.

Compile is not for scriptlets.

davidekholm commented 4 years ago

Who defines that? I'm sorry, but it's naturally a tremendous advantage to not textually parse the same scriptlet hundreds of times, when it can be parsed once and then executed multiple times.

nickl- commented 4 years ago

@davidekholm I will do this for you...

Did an impact analysis and some benchmarks and the cost of checking line endings is marginal however the price of additional empty lines does not come for free.

Stumbled on a different disaster that relates to the compiled scripts implementation which puts everything in a bit of a mess at the moment. It is going to take a little longer...

Thank you for your patience!

Open: will implement

davidekholm commented 4 years ago

Happy to hear that you're willing to allow this. From a user perspective, It's ideal if one only needs to handle the caching of the compiled scritplets but can otherwise enjoy the benefits of compiled code without the need to rewrite the end user code.

nickl- commented 4 years ago

@davidekholm All done!

You should use the serial garbage collector with java command line argument -XX:+UseSerialGC because the parallel garbage collector chokes trying to allocate new eden capacity.

Managed some major optimisations with test time to completion reduced by about 8 seconds overall.

The following jmh benchmark:

import org.openjdk.jmh.annotations.*;
import bsh.Interpreter;
import bsh.PreparsedScript;
import java.util.Collections;

@State(Scope.Benchmark)
public class MyBenchmark {

    Interpreter bsh = new Interpreter();
    PreparsedScript pscr;
    Integer val = 9;

    public MyBenchmark() {
        try {
            pscr = new PreparsedScript("4+5;");
        } catch(Throwable t) {
            throw new RuntimeException(t);
        }
    }

    @Benchmark
    public void test_eval() throws Throwable  {
       if ( !val.equals(bsh.eval("4+5;")) )
           throw new Exception("Incorrect value");
    }

    @Benchmark
    public void test_prepared() throws Throwable  {
        if ( !val.equals(pscr.invoke(Collections.EMPTY_MAP)) )
           throw new Exception("Incorrect value");
    }
}

Originally could not be completed by prepared/compiled scripts with application stalling to a halt.

Benchmark                  Mode  Cnt    Score     Error  Units
MyBenchmark.test_eval      avgt    5   23.783 ±   0.767  us/op
MyBenchmark.test_prepared  avgt    5  417.903 ± 494.772  us/op

Then managed to get things squared up again to produce satisfactory results over the weekend.

Benchmark                  Mode  Cnt   Score   Error  Units
MyBenchmark.test_eval      avgt    5  23.618 ± 0.659  us/op
MyBenchmark.test_prepared  avgt    5  16.676 ± 1.839  us/op

Now after all the additional optimisations, you can hardly believe it is the same test.

Benchmark                  Mode  Cnt  Score   Error  Units
MyBenchmark.test_eval      avgt    5  9.279 ± 1.083  us/op
MyBenchmark.test_prepared  avgt    5  3.316 ± 0.301  us/op

The score is read in microseconds per operation, error shows the deviation between slowest and fastest over 5 iterations.

davidekholm commented 4 years ago

Awesome Nick! Thanks for these fixes.