Improve Embedding.ConvertToVectorOfFloats #38
Merged
Conversation
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
It's currently incorrect on big endian systems (e.g. IBM s390x), as it blits the little-endian bytes into an array of floats. On big endian the values need to be reversed. It also assumes the data being sent back from the service is always correct. If it's corrupted in certain ways, such as not being as long as was expected, not being quoted, etc., we can silently get bad data. It also more allocation than is necessary. It first allocates a string for the base64 string data. Then it allocates a new string with the quotes stripped off. Then it allocates a byte[] with the decoded bytes. And finally it allocates the resulting float[]. We can avoid the initial string by getting the raw memory from the BinaryData. We can avoid the substring by just slicing those bytes. And we can generally avoid the temporary byte[] by renting one from the array pool. That leaves just the required float[].
KrzysztofCwalina
approved these changes
Jun 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
It's currently incorrect on big endian systems (e.g. IBM s390x), as it blits the little-endian bytes into an array of floats. On big endian the values need to be reversed.
It also assumes the data being sent back from the service is always correct. If it's corrupted in certain ways, such as not being as long as was expected, not being quoted, etc., we can silently get bad data.
It also more allocation than is necessary. It first allocates a string for the base64 string data. Then it allocates a new string with the quotes stripped off. Then it allocates a byte[] with the decoded bytes. And finally it allocates the resulting float[]. We can avoid the initial string by getting the raw memory from the BinaryData. We can avoid the substring by just slicing those bytes. And we can generally avoid the temporary byte[] by renting one from the array pool. That leaves just the required float[].