syscall: UTF16FromString is allocating more memory than needed

[freddy77]

Using Golang profile tools it looked the function was taking quite a lot of memory so I looked at the code.

What version of Go are you using (go version)?

$ go version
go version devel go1.21-f742ddc Wed May 3 22:13:51 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""

GOARCH="amd64"

GOBIN=""

GOCACHE="/home/fziglio/.cache/go-build"

GOENV="/home/fziglio/.config/go/env"

GOEXE=".exe"

GOEXPERIMENT=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="linux"

GOINSECURE=""

GOMODCACHE="/home/fziglio/go/pkg/mod"

GONOPROXY="repo.jazzdev.io"

GONOSUMDB="repo.jazzdev.io"

GOOS="windows"

GOPATH="/home/fziglio/go"

GOPRIVATE="repo.jazzdev.io"

GOPROXY="https://proxy.golang.org/,direct"

GOROOT="/home/fziglio/work/go"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/home/fziglio/work/go/pkg/tool/linux_amd64"

GOVCS=""

GOVERSION="devel go1.21-f742ddc Wed May 3 22:13:51 2023 +0000"

GCCGO="gccgo"

GOAMD64="v1"

AR="ar"

CC="gcc"

CXX="g++"

CGO_ENABLED="0"

GOMOD="/home/fziglio/work/go/src/go.mod"

GOWORK=""

CGO_CFLAGS="-O2 -g"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-O2 -g"

CGO_FFLAGS="-O2 -g"

CGO_LDFLAGS="-O2 -g"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-m64 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build939632449=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Discovered from a profile, looking at code. Just in case I wrote a test.

What did you expect to see?

Less memory usage

What did you see instead?

More memory usage than expected

[bcmills]

Please add more detail. How can we reproduce the issue to verify a fix?

[freddy77]

Please add more detail. How can we reproduce the issue to verify a fix?

I mainly opened this issue in order to post a patch. From the code it's clear the issue, see https://go-review.googlesource.com/c/go/+/492575. I probably failed to link the issue to the PR.

[qmuntal]

Please add more detail. How can we reproduce the issue to verify a fix?

Agree that a more descriptive issue will help to better document this issue for future readers.

Some context: I optimized the syscall.UTF16FromString heap allocations in CL 425054 by switching to utf16.AppendRune, which works best if the backing slice is preallocated with a meaningful value.

By choosing len(s)*2+1, it ensures that UTF16FromString will only allocate once, as at most each UTF-8 character require two uint16. On the other hand, UTF-8 characters can normally be mapped to just one uint16, so it means that we are overallocating bytes in the normal case to save an allocation in the worst case.

@freddy77 might be right, it would be better to optimize for the normal case than for the worst-case, which at the same time is less common.

[gopherbot]

Change https://go.dev/cl/492575 mentions this issue: syscall: reduce memory allocated by UTF16FromString

[freddy77]

Please add more detail. How can we reproduce the issue to verify a fix?

Agree that a more descriptive issue will help to better document this issue for future readers.

Some context: I optimized the syscall.UTF16FromString heap allocations in CL 425054 by switching to utf16.AppendRune, which works best if the backing slice is preallocated with a meaningful value.

By choosing len(s)*2+1, it ensures that UTF16FromString will only allocate once, as at most each UTF-8 character require two uint16. On the other hand, UTF-8 characters can normally be mapped to just one uint16, so it means that we are overallocating bytes in the normal case to save an allocation in the worst case.

@freddy77 might be right, it would be better to optimize for the normal case than for the worst-case, which at the same time is less common.

No, I'm not optimizing the common case but making worse for the worst cases, the previous formulae, considering the comment, was confusing bytes and characters. len(s) is returning the length in bytes, not characters. The maximum ratio code units/bytes is 1, not 2.

[qmuntal]

No, I'm not optimizing the common case but making worse for the worst cases, the previous formulae, considering the comment, was confusing bytes and characters. len(s) is returning the length in bytes, not characters. The maximum ratio code units/bytes is 1, not 2.

Doh, I thought len(s) returned the number of characters, not bytes. Then len(s)*2+1 is overallocating in all cases. Thanks for the correction.