mparlak / Flee

Fast Lightweight Expression Evaluator
629 stars 120 forks source link

ExpressionContext exception in constructor #73

Open stefanegon opened 5 years ago

stefanegon commented 5 years ago

When creating a new instance of ExpressionContext, the following exceptions are being thrown:

Flee.Parsing.grammatica_1._5.alpha2.PerCederberg.Grammatica.Runtime.RE.RegExpException: 'invalid repeat count: '{1,3})?(d|f|m)?' at position 18'

Flee.Parsing.grammatica_1._5.alpha2.PerCederberg.Grammatica.Runtime.RE.RegExpException: 'invalid repeat count: '{4}|\[\"'trn])*"' at position 24'

Flee.Parsing.grammatica_1._5.alpha2.PerCederberg.Grammatica.Runtime.RE.RegExpException: 'invalid repeat count: '{2}:\d{2}(:\d{2}(.\d{1,7})?)?#' at position 12'

Can those be fixed? Using the latest nuget package.

Thanks

angelosaiani commented 4 years ago

The same thing happens to me too. Can you help us and release a fix! Thanks

Soloresq commented 4 years ago

Same here!

Octoate commented 4 years ago

The same happens to me and has a performance impact on the software since I have to instantiate a lot of instances of it. A fix for this would be awesome.

Suplanus commented 4 years ago

Same here!

giadasql commented 4 years ago

I have the same issue, can this be fixed? Thank you!

tatejones commented 4 years ago

Did anyone find a solution for this (pull request)? I noticed this related comment, reference to flaw in the NFA matcher's use of TokenRegExpParser.ParseAtomModifier

hunkydoryrepair commented 3 years ago

Did anyone find a solution for this (pull request)? I noticed this related comment, reference to flaw in the NFA matcher's use of TokenRegExpParser.ParseAtomModifier

As that person pointed out, it is in the matcher. The NFA is an efficient regex matcher, but it is limited. When the regular expression is too complicated for it to handle, it throws an exception and instead a full regex parser is used (either Grammatica or the system regex as an additional fallback).

IMO, if you are creating a new context repeatedly, you should expect a performance hit. There is a significant amount of work happening to set up the context, and of course that means no expressions can be reused, either. The use of Flee is best when the expressions are evaluated repeatedly, using the same context. Otherwise, there is no point in emitting IL in the first place, and other parsers would be a better fit.

Of course, the regular expressions that are too complex are known. A simple solution is to flag these expressions and skip any attempt to use NFA on them. While "exceptions hurt performance" is somewhat true, it appears there are 5 exceptions thrown when setting up an ExpressionContext, which is a pretty small number, costing about 0.00001s.

Under a debugger, you might have a more significant hit, if the settings are to break on exceptions, but this doesn't happen when executing. The performance issue should be addressed by reusing the ExpressionContext and wherever possible, reuse the Expression itself.