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

feat(pkg/parser): impl expression eval #54

Merged
merged 3 commits into from
Aug 26, 2022
Merged

feat(pkg/parser): impl expression eval #54

merged 3 commits into from
Aug 26, 2022

Conversation

WenyXu
Copy link
Contributor

@WenyXu WenyXu commented Aug 13, 2022

No description provided.

@WenyXu WenyXu requested review from nodece and noneback August 13, 2022 13:17
@WenyXu WenyXu changed the title feat(pkg/parser): impl experssion eval feat(pkg/parser): impl expression eval Aug 13, 2022
@WenyXu
Copy link
Contributor Author

WenyXu commented Aug 20, 2022

I compared the performance to the govaluate.

For the parsing performance, our implementation is about 20x-25x slower than the govaluate. In most cases, it's acceptable. We only parse the expression once when the expression is defined, and then the expression will be stored as an abstract syntax tree in the memory.

For the evaluation performance, In our scenarios, our implementation is about 2-8x faster than the govaluate. Notably, in the evaluation with parameters and functions, our implementation's performance is about 8x faster than the govaluate.

goos: linux
goarch: amd64
pkg: github.com/casbin-mesh/neo/pkg/parser
cpu: 12th Gen Intel(R) Core(TM) i7-12700
BenchmarkSingleParse_govaluate
BenchmarkSingleParse_govaluate-20                      	 1859112	       614.8 ns/op
BenchmarkSingleParse
BenchmarkSingleParse-20                                	  224336	     12154 ns/op
BenchmarkEvaluationSingle_govaluate
BenchmarkEvaluationSingle_govaluate-20                 	126624380	         8.701 ns/op
BenchmarkEvaluationSingle
BenchmarkEvaluationSingle-20                           	890921866	         1.361 ns/op
BenchmarkEvaluationNumericLiteral_govaluate
BenchmarkEvaluationNumericLiteral_govaluate-20         	30109729	        33.80 ns/op
BenchmarkEvaluationNumericLiteral
BenchmarkEvaluationNumericLiteral-20                   	121256878	         9.713 ns/op
BenchmarkEvaluationLiteralModifiers_govaluate
BenchmarkEvaluationLiteralModifiers_govaluate-20       	14240113	        83.52 ns/op
BenchmarkEvaluationLiteralModifiers
BenchmarkEvaluationLiteralModifiers-20                 	68422755	        15.39 ns/op
BenchmarkEvaluationParameters_govaluate
BenchmarkEvaluationParameters_govaluate-20             	17665630	        67.55 ns/op
BenchmarkEvaluationParameters
BenchmarkEvaluationParameters-20                       	37160481	        30.01 ns/op
BenchmarkEvaluationParametersModifiers_govaluate
BenchmarkEvaluationParametersModifiers_govaluate-20    	 9849256	       135.3 ns/op
BenchmarkEvaluationParametersModifiers
BenchmarkEvaluationParametersModifiers-20              	23305705	        51.33 ns/op
BenchmarkEvaluationFunction_govaluate
BenchmarkEvaluationFunction_govaluate-20               	 2959526	       405.5 ns/op
BenchmarkEvaluationFunction
BenchmarkEvaluationFunction-20                         	23955829	        53.09 ns/op

@WenyXu
Copy link
Contributor Author

WenyXu commented Aug 25, 2022

Update Benchmarks

I refactored the expression evaluation and fixed a bug that the code might mutate the value in the context during the expression's evaluation. I fixed that by introducing extra memory allocation during the evaluation, but it also reduced the performance.

BenchmarkEvaluationParametersModifiers_govaluate-20    	 7920292	       134.4 ns/op
BenchmarkEvaluationParametersModifiers-20              	 5718110	       185.6 ns/op

As you can see, when the evaluation only involves parameters, the performance was slightly worse than the govalute because it spent about 20% CPU time to allocate memory and copy the primitive's value.

BenchmarkEvaluationFunction_govaluate-20               	 2948817	       410.3 ns/op
BenchmarkEvaluationFunction-20                         	13911880	        92.45 ns/op
BenchmarkEvaluationFunctionOnly-20                     	19744444	        60.46 ns/op
BenchmarkEvaluationFunction_Naive-20                   	1000000000	         0.1200 ns/op

However, we still can be benefited in some scenarios. If the evaluation only involves functions, there are no allocation or copy operations.

goos: linux
goarch: amd64
pkg: github.com/casbin-mesh/neo/pkg/expression
cpu: 12th Gen Intel(R) Core(TM) i7-12700
BenchmarkSingleParse_govaluate
BenchmarkSingleParse_govaluate-20                      	 2228401	       619.7 ns/op
BenchmarkSingleParse
BenchmarkSingleParse-20                                	  126939	     13510 ns/op
BenchmarkEvaluationSingle_govaluate
BenchmarkEvaluationSingle_govaluate-20                 	131851587	         9.153 ns/op
BenchmarkEvaluationSingle
BenchmarkEvaluationSingle-20                           	770851652	         1.405 ns/op
BenchmarkEvaluationNumericLiteral_govaluate
BenchmarkEvaluationNumericLiteral_govaluate-20         	34579762	        35.61 ns/op
BenchmarkEvaluationNumericLiteral
BenchmarkEvaluationNumericLiteral-20                   	124814935	         9.716 ns/op
BenchmarkEvaluationLiteralModifiers_govaluate
BenchmarkEvaluationLiteralModifiers_govaluate-20       	14283465	        82.40 ns/op
BenchmarkEvaluationLiteralModifiers
BenchmarkEvaluationLiteralModifiers-20                 	74358448	        15.80 ns/op
BenchmarkEvaluationParameters_govaluate
BenchmarkEvaluationParameters_govaluate-20             	19192678	        62.81 ns/op
BenchmarkEvaluationParameters
BenchmarkEvaluationParameters-20                       	 9405704	       128.8 ns/op
BenchmarkEvaluationParametersModifiers_govaluate
BenchmarkEvaluationParametersModifiers_govaluate-20    	 7920292	       134.4 ns/op
BenchmarkEvaluationParametersModifiers
BenchmarkEvaluationParametersModifiers-20              	 5718110	       185.6 ns/op
BenchmarkEvaluationFunction_govaluate
BenchmarkEvaluationFunction_govaluate-20               	 2948817	       410.3 ns/op
BenchmarkEvaluationFunction
BenchmarkEvaluationFunction-20                         	13911880	        92.45 ns/op
BenchmarkEvaluationFunction_Naive
BenchmarkEvaluationFunction_Naive-20                   	1000000000	         0.1200 ns/op
BenchmarkEvaluationFunctionOnly
BenchmarkEvaluationFunctionOnly-20                     	19744444	        60.46 ns/op

@hsluoyz
Copy link

hsluoyz commented Aug 26, 2022

@nodece @noneback plz review

@noneback noneback self-requested a review August 26, 2022 15:59
@noneback noneback merged commit 291df80 into main Aug 26, 2022
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

Successfully merging this pull request may close these issues.

3 participants