Files
moby/pkg/stringid/stringid.go
Sebastiaan van Stijn 0539b7073e pkg/stringid: optimize GenerateRandomID
GenerateRandomID has a check to verify if the generated ID was numeric. This
check was added because a container's short-ID is used as default hostname for
containers, which isn't allowed to be consisting of only numbers (see [moby#3869]
and https://bugzilla.redhat.com/show_bug.cgi?id=1059122.

Producing an random ID with only numbers is a rare corner-case, but the check
would always be executed and wasn't optimized.

This patch applies some optimizations:

- The code was using `strconv.ParseUInt`, which has additional checks for
  signs ("+" or "-"); `hex.EncodeToString` would never produce these, so
  we can use `strconv.ParseInt` instead (which doesn't have these checks).
- The code was using `TruncateID(id)` to get the short-ID. The `TruncateID`
  function is designed to also handle digests, and for that checks for
  the given ID to contain colons (`:`), which it would split to remove
  the algorithm (`sha256:`) before truncating to the short-ID length.
  That check wasn't needed either, because those would not be produced
  by `hex.EncodeToString`, so instead, we can just truncate the ID.
- Finally, all we _really_ need to check for is if the ID consists of only
  numeric characters (`0-9`) so, let's do just that; if any non-numeric
  value is found, the ID is valid, and we can terminate the loop.

I did some basic benchmark to compare all of the above in isolation;

- BenchmarkParseInt: `strconv.ParseInt(TruncateID(id), 10, 64)`
- BenchmarkParseUInt: `strconv.ParseUint(TruncateID(id), 10, 64)`
- BenchmarkParseUIntNoTrunc: `strconv.ParseUint(id[:shortLen], 10, 64)`
- BenchmarkAllNum: `allNum(id[:shortLen])`

Results of the above:

    BenchmarkParseInt-10                1713937       691.0 ns/op     480 B/op      18 allocs/op
    BenchmarkParseIntNoTrunc-10         3385483       356.1 ns/op     480 B/op      18 allocs/op
    BenchmarkParseUInt-10               2112538       567.7 ns/op     384 B/op      12 allocs/op
    BenchmarkParseUIntNoTrunc-10        4325847       266.7 ns/op     384 B/op      12 allocs/op
    BenchmarkAllNum-10                 77277264        15.29 ns/op      0 B/op       0 allocs/op

Difference for `GenerateRandomID` as a whole is less dramatic, as in most
cases `ParseInt` would bail out early, but still saves some allocations, and
performance is ~14% better:

    BenchmarkGenerateRandomID-10        2807764       424.5 ns/op     240 B/op       6 allocs/op
    BenchmarkGenerateRandomIDNew-10     3288866       366.6 ns/op     160 B/op       3 allocs/op

[moby#3869]: https://github.com/moby/moby/issues/3869

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-21 11:17:05 +02:00

64 lines
1.6 KiB
Go

// Package stringid provides helper functions for dealing with string identifiers
package stringid // import "github.com/docker/docker/pkg/stringid"
import (
"crypto/rand"
"encoding/hex"
"strings"
)
const (
shortLen = 12
fullLen = 64
)
// TruncateID returns a shorthand version of a string identifier for convenience.
// A collision with other shorthands is very unlikely, but possible.
// In case of a collision a lookup with TruncIndex.Get() will fail, and the caller
// will need to use a longer prefix, or the full-length Id.
func TruncateID(id string) string {
if i := strings.IndexRune(id, ':'); i >= 0 {
id = id[i+1:]
}
if len(id) > shortLen {
id = id[:shortLen]
}
return id
}
// GenerateRandomID returns a unique, 64-character ID consisting of a-z, 0-9.
// It guarantees that the ID, when truncated ([TruncateID]) does not consist
// of numbers only, so that the truncated ID can be used as hostname for
// containers.
func GenerateRandomID() string {
b := make([]byte, 32)
for {
if _, err := rand.Read(b); err != nil {
panic(err) // This shouldn't happen
}
id := hex.EncodeToString(b)
// make sure that the truncated ID does not consist of only numeric
// characters, as it's used as default hostname for containers.
//
// See:
// - https://github.com/moby/moby/issues/3869
// - https://bugzilla.redhat.com/show_bug.cgi?id=1059122
if allNum(id[:shortLen]) {
// all numbers; try again
continue
}
return id
}
}
// allNum checks whether id consists of only numbers (0-9).
func allNum(id string) bool {
for _, c := range []byte(id) {
if c > '9' || c < '0' {
return false
}
}
return true
}