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

Improve performance of FromObject and parseUUID #129

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sgmiller
Copy link

@sgmiller sgmiller commented Oct 24, 2022

FromObject had a bug where it was attempting to preallocate a slice with
enough capacity to avoid growth, by calculating length^3. But ^ is XOR, so
it was allocation an empty slice (or at least much smaller than expected).

parseUUID is a bit of a odd function, parsing partial UUIDs as well as
correct ones. Simplified it to improve perf by 60%.

Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

Awesome. LGTM.

I checked the UUID index tests which do an OK job of testing different cases we expect to parse or not and this passes so seems like a solid improvement!

Is it worth adding a case to the existing test that exercises the invalid Hex path? It doesn't seem to have any input that would make it through hexToByte with an invalid input?

arg += "\x00"
return []byte(arg), nil

// Convert to a []byte with a trailing nul byte
Copy link

Choose a reason for hiding this comment

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

typo:

Suggested change
// Convert to a []byte with a trailing nul byte
// Convert to a []byte with a trailing null byte

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