enhance #10 : avoid allocations on parseUUID#12
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Flags with carried forward coverage won't be shown. Click here to find out more.
|
parseUUID
| if len(uuid) == 32 { | ||
| // Fast path for UUIDs without dashes | ||
| return hex.DecodeString(uuid) | ||
| } else if len(uuid) == 36 { | ||
| // Validate dash positions | ||
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | ||
| return nil, errors.New("invalid UUID format") | ||
| } | ||
| } else { | ||
| return nil, errors.New("invalid UUID length") | ||
| } |
There was a problem hiding this comment.
| if len(uuid) == 32 { | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) | |
| } else if len(uuid) == 36 { | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| } else { | |
| return nil, errors.New("invalid UUID length") | |
| } | |
| switch len(uuid) { | |
| case 32: | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) | |
| case 36: | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| default: | |
| return nil, errors.New("invalid UUID length") | |
| } |
There was a problem hiding this comment.
Also, if you consider this
| if len(uuid) == 32 { | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) | |
| } else if len(uuid) == 36 { | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| } else { | |
| return nil, errors.New("invalid UUID length") | |
| } | |
| switch len(uuid) { | |
| case 32: | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) | |
| case 36: | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| return hex.DecodeString(strings.ReplaceAll(uuid, "-", "")) | |
| default: | |
| return nil, errors.New("invalid UUID length") | |
| } |
There was a problem hiding this comment.
Or this
| if len(uuid) == 32 { | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) | |
| } else if len(uuid) == 36 { | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| } else { | |
| return nil, errors.New("invalid UUID length") | |
| } | |
| if len(uuid) == 36 { | |
| // Validate dash positions | |
| if uuid[8] != '-' || uuid[13] != '-' || uuid[18] != '-' || uuid[23] != '-' { | |
| return nil, errors.New("invalid UUID format") | |
| } | |
| uid = strings.ReplaceAll(uid, "-", "") | |
| } | |
| if len(uuid) != 32 { | |
| return nil, errors.New("invalid UUID length") | |
| } | |
| // Fast path for UUIDs without dashes | |
| return hex.DecodeString(uuid) |
| // Remove dashes while copying characters | ||
| result := make([]byte, 32) | ||
| j := 0 | ||
| for i := 0; i < len(uuid); i++ { | ||
| if uuid[i] == '-' { | ||
| continue | ||
| } | ||
| result[j] = uuid[i] | ||
| j++ | ||
| } |
There was a problem hiding this comment.
Why not a simple strings.ReplaceAll?
There was a problem hiding this comment.
Good point! Primary reason was to address #10 . When I attempted 'Benchmarking', the current implementation was faster and avoids memory allocations.
Here are the benchmark results comparing the two:
| Implementation | Time per Operation (ns/op) | Speed |
|---|---|---|
strings.ReplaceAll() |
146.2 ns/op | Baseline |
| Current Approach | 68.17 ns/op | 2.22x faster |
How?
This approach processes the UUID in one pass, directly copying characters into a pre-allocated byte slice. This avoids creating a new string, which is what strings.ReplaceAll does and keeps things lean.
| "123", // Too short | ||
| "123e4567e89b12d3a4564266141740000000", // Too long | ||
| "123e4567e89b12d3a45642661417400g", // Invalid character | ||
| "123e-4567-e89b-12d3-a456-426614174000", // Misplaced dashes |
There was a problem hiding this comment.
Please add:
- a valid uid with the dashes at the right place
- an invalid uid with the dash at the right place plus one randomly placed,or simply 36 dashes
There was a problem hiding this comment.
Sure thing! Added in the latest commit.
|
is this changes in some parts goes to google/uuid repo pr ? |
|
Let me see what I can do best there. I had the flexibility of writing this repo in my preferred style and structure. With |
|
Sadly, people aren't reviewing the proposal there. It'd be great if the PR gets some attention. |
Changes
With this PR, we rewrite
parseUUIDto process input directly, avoiding unnecessary string allocations when handling UUIDs with-. The function now uses a pre-allocated byte slice and skips dashes inline to improve its efficiency, with few additional checks.This should address the issue raised in #10. Let me know your thoughts! @vtolstov