Skip to content

FFM-11115 - Update Evaluation logic to be more efficient #147

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

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

stephenmcconkey
Copy link
Contributor

@stephenmcconkey stephenmcconkey commented Apr 3, 2024

What
Updates Go SDK Evaluation logic around clauses
With this we remove the need to use reflect in this logic

Why
There are Performance increases to be gained from doing this, which FF-Server would benefit from doing client side evaluations

Testing
EvaluateClause tests passed as before so same input is bringing same output
Fixed Tests for the getAttr function to now return a string (as we could remove the reflect to string func)
Benchmark Testing as below

Evidence

Refactored Code:

/Users/stephenmcconkey/Library/Caches/JetBrains/GoLand2023.2/tmp/GoLand/___gobench_github_com_harness_ff_golang_server_sdk_evaluation.test -test.v -test.paniconexit0 -test.bench . -test.run ^$
goos: darwin
goarch: arm64
pkg: github.com/harness/ff-golang-server-sdk/evaluation
BenchmarkEvaluateClause_NilClause
BenchmarkEvaluateClause_NilClause-10                 	539388770	         2.118 ns/op
BenchmarkEvaluateClause_EmptyOperator
BenchmarkEvaluateClause_EmptyOperator-10             	573256963	         2.147 ns/op
BenchmarkEvaluateClause_NilValues
BenchmarkEvaluateClause_NilValues-10                 	571950588	         2.085 ns/op
BenchmarkEvaluateClause_EmptyValues
BenchmarkEvaluateClause_EmptyValues-10               	577386420	         2.076 ns/op
BenchmarkEvaluateClause_WrongOperator
BenchmarkEvaluateClause_WrongOperator-10             	95167285	        12.60 ns/op
BenchmarkEvaluateClause_EmptyAttribute
BenchmarkEvaluateClause_EmptyAttribute-10            	237732388	         5.073 ns/op
BenchmarkEvaluateClause_MatchOperator
BenchmarkEvaluateClause_MatchOperator-10             	93533827	        12.94 ns/op
BenchmarkEvaluateClause_MatchOperatorError
BenchmarkEvaluateClause_MatchOperatorError-10        	90778142	        12.88 ns/op
BenchmarkEvaluateClause_InOperator
BenchmarkEvaluateClause_InOperator-10                	96405542	        12.55 ns/op
BenchmarkEvaluateClause_InOperatorNotFound
BenchmarkEvaluateClause_InOperatorNotFound-10        	94263358	        12.75 ns/op
BenchmarkEvaluateClause_EqualOperator
BenchmarkEvaluateClause_EqualOperator-10             	93184884	        12.94 ns/op
BenchmarkEvaluateClause_EqualSensitiveOperator
BenchmarkEvaluateClause_EqualSensitiveOperator-10    	93156548	        12.93 ns/op
BenchmarkEvaluateClause_GTOperator
BenchmarkEvaluateClause_GTOperator-10                	95136472	        12.60 ns/op
BenchmarkEvaluateClause_GTOperatorNegative
BenchmarkEvaluateClause_GTOperatorNegative-10        	95507493	        12.60 ns/op
BenchmarkEvaluateClause_StartsWithOperator
BenchmarkEvaluateClause_StartsWithOperator-10        	93169813	        12.89 ns/op
BenchmarkEvaluateClause_EndsWithOperator
BenchmarkEvaluateClause_EndsWithOperator-10          	94167802	        12.93 ns/op
BenchmarkEvaluateClause_ContainsOperator
BenchmarkEvaluateClause_ContainsOperator-10          	92775730	        12.91 ns/op
BenchmarkEvaluateClause_SegmentMatchOperator
BenchmarkEvaluateClause_SegmentMatchOperator-10      	238270216	         5.027 ns/op
PASS

Old Code:

/Users/stephenmcconkey/Library/Caches/JetBrains/GoLand2023.2/tmp/GoLand/___gobench_github_com_harness_ff_golang_server_sdk_evaluation.test -test.v -test.paniconexit0 -test.bench . -test.run ^$
goos: darwin
goarch: arm64
pkg: github.com/harness/ff-golang-server-sdk/evaluation
BenchmarkEvaluateClause_NilClause
BenchmarkEvaluateClause_NilClause-10                 	549836156	         2.072 ns/op
BenchmarkEvaluateClause_EmptyOperator
BenchmarkEvaluateClause_EmptyOperator-10             	578069950	         2.067 ns/op
BenchmarkEvaluateClause_NilValues
BenchmarkEvaluateClause_NilValues-10                 	577941537	         2.088 ns/op
BenchmarkEvaluateClause_EmptyValues
BenchmarkEvaluateClause_EmptyValues-10               	573185658	         2.101 ns/op
BenchmarkEvaluateClause_WrongOperator
BenchmarkEvaluateClause_WrongOperator-10             	32965221	        35.93 ns/op
BenchmarkEvaluateClause_EmptyAttribute
BenchmarkEvaluateClause_EmptyAttribute-10            	85589721	        14.03 ns/op
BenchmarkEvaluateClause_MatchOperator
BenchmarkEvaluateClause_MatchOperator-10             	33532392	        35.96 ns/op
BenchmarkEvaluateClause_MatchOperatorError
BenchmarkEvaluateClause_MatchOperatorError-10        	32738320	        36.24 ns/op
BenchmarkEvaluateClause_InOperator
BenchmarkEvaluateClause_InOperator-10                	32440322	        36.04 ns/op
BenchmarkEvaluateClause_InOperatorNotFound
BenchmarkEvaluateClause_InOperatorNotFound-10        	33301380	        36.36 ns/op
BenchmarkEvaluateClause_EqualOperator
BenchmarkEvaluateClause_EqualOperator-10             	33263110	        36.21 ns/op
BenchmarkEvaluateClause_EqualSensitiveOperator
BenchmarkEvaluateClause_EqualSensitiveOperator-10    	33258616	        36.45 ns/op
BenchmarkEvaluateClause_GTOperator
BenchmarkEvaluateClause_GTOperator-10                	32981869	        36.60 ns/op
BenchmarkEvaluateClause_GTOperatorNegative
BenchmarkEvaluateClause_GTOperatorNegative-10        	33067188	        35.88 ns/op
BenchmarkEvaluateClause_StartsWithOperator
BenchmarkEvaluateClause_StartsWithOperator-10        	33081355	        36.06 ns/op
BenchmarkEvaluateClause_EndsWithOperator
BenchmarkEvaluateClause_EndsWithOperator-10          	33321454	        36.29 ns/op
BenchmarkEvaluateClause_ContainsOperator
BenchmarkEvaluateClause_ContainsOperator-10          	33318834	        36.32 ns/op
BenchmarkEvaluateClause_SegmentMatchOperator
BenchmarkEvaluateClause_SegmentMatchOperator-10      	84904160	        14.06 ns/op
PASS

Summary of results is:

  • NilClause, EmptyOperator, NilValues, EmptyValues: Performance in these categories is very similar between the new and old code. The new code shows a very slight increase in execution time per operation, indicating a negligible performance impact.
  • WrongOperator, MatchOperator, MatchOperatorError, InOperator, InOperatorNotFound, EqualOperator, EqualSensitiveOperator, GTOperator, GTOperatorNegative, StartsWithOperator, EndsWithOperator, ContainsOperator: Significant performance improvement in the refactored code. The execution time per operation has decreased dramatically from around 35-36 ns/op to approximately 12-13 ns/op, indicating that the new code is nearly 3 times faster for these operations.
  • EmptyAttribute, SegmentMatchOperator: also show improvement in the refactored code. Execution time has reduced from around 14 ns/op to about 5 ns/op, more than halving the time taken per operation, which is a considerable improvement.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@erdirowlands erdirowlands left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 excellent removing reflection where it's not needed.

@stephenmcconkey
Copy link
Contributor Author

Tests passed on Testgrid running locally.

Holding off on merge until next week

Screenshot 2024-04-04 at 16 34 56

@stephenmcconkey stephenmcconkey merged commit 1780158 into main Apr 9, 2024
3 checks passed
@stephenmcconkey stephenmcconkey deleted the FFM-11115 branch April 9, 2024 09:11
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.

None yet

3 participants