Avoid creating a new ExpressionParser every time Parser::parse() is used

[fgalan]

Problem

Current jex-rs implementation creates a new ExpressionParser each time Paser::parse() method is used, i.e.:

impl Parser {
    pub fn parse(input: &str) -> Result<ast::Expression, ParseError<usize, Token, &str>> {
        Ok(*parser::ExpressionParser::new().parse(input)?)
    }
}

We have empirically checked (using a program with 1000 threads, each one doing a parse() call every 2 seconds) this is pretty inefficient, almost blocking our testing system (top showed cpu load 150-200%).

Digging into this with a profiler we discovered that most of the time is wasted in regex compilation (which makes sense, as I understand creating of a new ExpressionParser involves creating a regex "codifying" the parsing rules).

Solution

Instead of creating a new ExpressionParser every time Parser::parse() is called, this PR proposes to create the ExpressionParser at Parser construction time (i.e. at new()) in an internal parser variable, then use that internal variable each time Parser::parse() is called.

impl Parser {
    pub fn new() -> Self {
        Parser {
            parser: parser::ExpressionParser::new()
        }
    }

    pub fn parse<'a>(& self, input: &'a str) -> Result<ast::Expression, ParseError<usize, Token<'a>, &'static str>> {
        Ok(*self.parser.parse(input)?)
    }
}

Note this involves changes in the signature in the Evaluator::eval_in_context() method to provide the Parser as argument. TheEvaluator::eval() method has not been changed, but probably should be done in the same way (see FIXME mark in the PR).

It would be desirable to avoid that signature changes (in order not breaking backward compatibility in existing clients of this library), for instance with a singleton pattern in Parser. However, we haven't been able to apply that pattern, as we are not experts in Rust (although a draft attempt is in this branch).

Please fell free of taking this PR and that draft as input to improve the solution if you want.

Contributors:

@fgalan
@mrutid

[fgalan]

Please consider the merging of this PR into main branch (and create a new release in https://crates.io/crates/jexl-eval after that :).

Any feedback is highly welcomed and appreciated!

[brennie]

We should make the parser be an argument to ::new() and add it as a field on the evaluator. This will be a breaking change, but its a better design.

[fgalan]

Thank you very much for you feedback! We'll have a look and come back...