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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: avoid allocation when assigning a non-pointer value to an interface that already contains a value of the same type #65348

Closed
nanokatze opened this issue Jan 29, 2024 · 6 comments
Closed

cmd/compile: avoid allocation when assigning a non-pointer value to an interface that already contains a value of the same type #65348

nanokatze opened this issue Jan 29, 2024 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@nanokatze
Copy link

nanokatze commented Jan 29, 2024 •
edited

Go version

go1.22rc1 or older

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/nanokatze/.cache/go-build'
GOENV='/home/nanokatze/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/nanokatze/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/nanokatze/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/nanokatze/code/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/nanokatze/code/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22rc1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1416462612=/tmp/go-build -gno-record-gcc-switches'

What did you do?

package main

import "testing"

type Big struct {
        A, B int
}

var p interface{} = Big{}

func BenchmarkInterfaceAssignment(b *testing.B) {
        b.ReportAllocs()

        for i := 0; i < b.N; i++ {
                p = Big{A: i, B: p.(Big).A}
        }
}

What did you see happen?

Storing a value of sufficiently-big type into an interface{}-typed variable unconditionally allocates.

What did you expect to see?

Storing even a big-typed value into an interface{}-typed variable should not allocate when the said variable already contains a value of the same big type. The resulting code should check that the type of the value in the interface is the same and overwrite the pointed-at value.
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 29, 2024
@mknyszek
Copy link
Contributor

CC @golang/compiler

@mknyszek mknyszek modified the milestones: Backlog, Unplanned Jan 29, 2024
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 29, 2024
@randall77
Copy link
Contributor

I don't think this is possible to do safely. If T is a big type,

var x, y T
var i any = x // pack x into an empty interface
j := i // copy it
i = y // pack y into an empty interface

We can't just copy y into the already-allocated backing store, because that would affect the value stored in j.

@aclements
Copy link
Member

Unfortunately, this won't work. Consider https://go.dev/play/p/K7qyjdoYQ77, which is similar to your example, except that it makes a copy of p before modifying it. After modifying p, the value of the copy must not have changed, which means we must have two Big objects in memory at that point. Hence, either copying p or modifying p must allocate for the second Big object.

@mknyszek
Copy link
Contributor

Based on the comments from @randall77 and @aclements, closing.

@mknyszek mknyszek closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2024
@aclements
Copy link
Member

If we had a way to determine there was a single reference to the interface's value, then we could do this in that case. We don't currently have any sort of analysis like that, either static or dynamic. One could imagine doing this statically, but I think it would apply in extremely limited circumstances (certainly not in the example given). We are actually exploring the possibility of doing dynamic single-reference, but that's at very early stages and it's not at all clear anything will come of it (I'm also not sure it will actually let us do these sorts of in-place update optimizations, though it would be nice if it did).

@nanokatze
Copy link
Author

Right, I didn't think of these examples, I now see why it wouldn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@mknyszek @aclements @randall77 @gopherbot @nanokatze