Optimize evaluation of treatments#210
Open
cltnschlosser wants to merge 7 commits intosplitio:mainfrom
Open
Conversation
The go-toolkit set internally has interface{} key, which prohibts the go compiler from making optimizations that it can for string keyed maps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
These changes resulted in a 3x performance increase of feature evaluation in a golang service I maintain. Almost every commit here can stand alone, and this PR can be broken up into individual (sometimes sequential) PRs for each commit if that makes it easier to review / merge.
The change with the largest impact is the first commit, introducing a layer for producing
grammar.Splitinstances for the evaluator to use instead of interacting directly with the storage layer. With in memory storage we can introduce a cache which significantly optimizes feature evaluation performance, because Matchers and Conditions don't need to be created for each evaluation. This change would be a semver major change to this library but the changes to splitio/go-client are straightforward and don't impact the api there (cltnschlosser/go-client@7acbaae). I have those changes made and can open a PR there after this is merged and there is a public version to use ingo.mod.I did increase the go version here to be able to take advantage of iterators, though if that's an issue I can introduce a version which creates a temporary map or slice instead.
Important note: These are scaled for maximum readability, but aren't scaled relative to each other. This makes areas that didn't receive optimizations now look larger. They aren't slower, they are just now a larger percentage of the (much smaller) total time.
Aside about impression tracking
As you can see above, after these optimizations, the slowest piece is the impression tracking. I didn't investigate this area much because I believe my use case don't use impression data from the split sdk at all, so a future improvement would be to offer a mechanism to completely disable it.
I also think that once that is solved, we may see better performance by calling the individual TreatmentWithConfig function in a loop, because we can avoid several map creations. And the deduplication step in ValidateFeatureNames, which isn't a concern for my case.
But as I said I didn't dig in too far here, I wanted to focus on changes that didn't change functionality.