Avoid creating a new ExpressionParser every time Parser::parse() is used #32
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Current jex-rs implementation creates a new
ExpressionParser
each timePaser::parse()
method is used, i.e.: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 timeParser::parse()
is called, this PR proposes to create theExpressionParser
atParser
construction time (i.e. atnew()
) in an internalparser
variable, then use that internal variable each timeParser::parse()
is called.Note this involves changes in the signature in the
Evaluator::eval_in_context()
method to provide theParser
as argument. TheEvaluator::eval()
method has not been changed, but probably should be done in the same way (seeFIXME
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