Nerdsniped - GC2023 #2
Open
Conversation
This file contains 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
improvments inherently involve a change in function signature to allow the calling function to pass in the result slice and start location as input parameters. This allows the caller to do all the memory allocation. This helps us as the Tokenize method can pre-compute the final slice size before doing a single large allocation. The new function is unexported, while the main Trigram method remains exported to preserve the func signature. However, since this method must still allocate its own memory, there is no significant improvement here. What IS improved greater is the way Tokenize uses the unexported trigram function. There is also a removal of an unecessary if branch.
optimization first.
|
Sweet. Im going to do a benchmark of all of the PR's against my best effort and turn it into a blog post, and then merge the winner. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Attempting an improvement on the (use of the) Trigram function.
We do this by moving the slice allocation outside the trigram function.
Trigramsis only used byTokenize, and that function can actually compute the result slice size while cleaning up the input. Thetrigramfunction now takes the result slice and a location to start populating the output.Since this inherently changes the function signature, we unexport it, and export the
Trigrammethod as a wrapper for this unexported function that implements the old function signature. Since the exported method must behave as per the old code, it still needs to do its own memory alloc, and as a result does not see much improvement. Most of the improvement here is seen in theTokenizemethod.Benchmarks on
Tokenize(on my machine) show ~1.5x improvementUPDATE: credit to @Merovius for this idea
Second optimization: remove the alloc for a []rune slice in the trigram function by walking the string with 3 variables to keep track of a start, middle, and end index, using the utf8 package DecodeRune function to find an appropriate increment each iteration without the need to allocate the rune slice.
This brings the final time down to ~2.6us per call to
Tokenize, a ~4.2x improvment