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

Compiled BeanShell more strict with semi colons #529

Closed
davidekholm opened this issue Nov 21, 2018 · 12 comments
Closed

Compiled BeanShell more strict with semi colons #529

davidekholm opened this issue Nov 21, 2018 · 12 comments

Comments

@davidekholm
Copy link

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-
Copy link
Member

nickl- commented Jan 11, 2019

@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
Copy link
Author

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-
Copy link
Member

nickl- commented Jan 11, 2019

Understood, thank you for the explanation.

@davidekholm
Copy link
Author

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-
Copy link
Member

nickl- commented Jun 20, 2019

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

@nickl- nickl- closed this as completed Jun 20, 2019
@davidekholm
Copy link
Author

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-
Copy link
Member

nickl- commented Jun 20, 2019

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

Compile is not for scriptlets.

@davidekholm
Copy link
Author

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-
Copy link
Member

nickl- commented Jun 21, 2019

@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

@nickl- nickl- reopened this Jun 21, 2019
@davidekholm
Copy link
Author

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- nickl- closed this as completed in 7ddef7a Jun 25, 2019
@nickl-
Copy link
Member

nickl- commented Jun 25, 2019

@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
Copy link
Author

Awesome Nick! Thanks for these fixes.

pgiffuni pushed a commit to pgiffuni/beanshell that referenced this issue Jan 8, 2024
Simplified the implementation of the PreparsedScript class, simple interpreter instance apply class loader to interpreter, eval the script to retrieve the callable object and reference the prepared instance.
On invoke create the scoped name space and local interpreter, set the context variables invoke the prepared script and unwrap the results.
Added compound name resolution in context variable names.
Standardised the source file info and do not reveal the wrapped method name.
Added terminated script conversion to ensure statement is terminated fixes  beanshell#529
Fixed unit tests and enabled multi threaded test, added additional tests to illustrate compound name resolution.
Added serial garbage collector java vm argument to properly apply large eden algorithms which parallel gc will stall on.
Added an empty map and a map of key value pair list helper to TestUtil for supplying tests with data maps.
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

2 participants