Skip to content

cmd/compile: stack allocate string and slice headers when passed through non-escaping interfaces #23676

Closed
@dsnet

Description

@dsnet

Consider the following snippet:

func NewA(x ...interface{}) error {
	if len(x) > 0 {
		return errors.New(x[0].(string))
	}
	return nil
}
func NewB(s ...string) error {
	if len(s) > 0 {
		return errors.New(s[0])
	}
	return nil
}

var sink error

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewB(s)
	}
}

Running benchmarks on this, you find:

BenchmarkA-8   	20000000	        84.3 ns/op	      32 B/op	       2 allocs/op
BenchmarkB-8   	30000000	        42.9 ns/op	      16 B/op	       1 allocs/op

Version A is more costly by 1 extra allocation. This occurs because calling NewA has a hidden call to runtime.convT2Estring where the string header is allocated on the heap.

However, I contend that the compiler should be able to prove that this is unnecessary. The call to NewA is concrete, and so the compiler should able to prove that a string header passed into the variadic interfaces neither escaped nor is modified. When crafting the interface header, it should be able to directly point to the string header on the stack.

\cc @randall77 @neild

Activity

changed the title [-]cmd/compile: stack allocate string and slice headers when passed through interface[/-] [+]cmd/compile: stack allocate string and slice headers when passed through non-escaping interfaces[/+] on Feb 3, 2018
randall77

randall77 commented on Feb 3, 2018

@randall77
Contributor
cherrymui

cherrymui commented on Feb 4, 2018

@cherrymui
Member

the compiler should able to prove that a string header passed into the variadic interfaces neither escaped nor is modified.

The escape analysis does not track whether things are modified. So it cannot make an interface with its data field directly pointing to s.

That said, it is possible to have it point to a copy of s on stack. However, currently the escape analysis doesn't distinguish the interface and its underlying value, i.e. x[0].(string) escaping is seen as x[0] escaping, which causes the string header allocated on heap.

dsnet

dsnet commented on Feb 5, 2018

@dsnet
MemberAuthor

If the caller knew that that the last use of the string was the function call itself, then it wouldn't even need to make a copy on the stack.

cherrymui

cherrymui commented on Feb 6, 2018

@cherrymui
Member

If the caller knew that that the last use of the string was the function call itself, then it wouldn't even need to make a copy on the stack.

Once we fix the escape problem, this will probably not matter. s can be SSA'd. To make the interface it needs to store s to a temp string header on stack so its address can be taken. There will be only one string header on stack.

added
NeedsFixThe path to resolution is known, but the work has not been done.
on Mar 28, 2018
added this to the Go1.11 milestone on Mar 28, 2018
modified the milestones: Go1.11, Unplanned on May 18, 2018
rogpeppe

rogpeppe commented on May 20, 2020

@rogpeppe
Contributor

Isn't this issue a duplicate of #8618 ?

zigo101

zigo101 commented on Sep 30, 2021

@zigo101

It looks this is a problem of inlining:

package main

import "testing"

//go:noinline
func NewA(x ...interface{}) string {
	if len(x) == 0 {
		return ""
	}
	return x[0].(string)
}

//go:noinline
func NewB(x []interface{}) string {
	if len(x) == 0 {
		return ""
	}
	return x[0].(string)
}

//go:noinline
func NewC(x ...interface{}) string {
	return NewB(x)
}

var sink string

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewB([]interface{}{s})
	}
}

func BenchmarkC(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewC(s)
	}
}

Output:

BenchmarkA-4   	278548590	         4.154 ns/op	       0 B/op	       0 allocs/op
BenchmarkB-4   	280900167	         4.201 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	211030160	         5.666 ns/op	       0 B/op	       0 allocs/op

After removing the //go:noinline directive lines, the output becomes into:

BenchmarkA-4   	26151573	       101.1 ns/op	      16 B/op	       1 allocs/op
BenchmarkB-4   	418234288	         2.631 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	27927691	       118.6 ns/op	      16 B/op	       1 allocs/op
zigo101

zigo101 commented on Sep 30, 2021

@zigo101

And this is not a string specified problem:

package main

import "testing"

func NewA(x ...interface{}) int {
	if len(x) == 0 {
		return 0
	}
	return x[0].(int)
}

func NewB(x []interface{}) int {
	if len(x) == 0 {
		return 0
	}
	return x[0].(int)
}

func NewC(x ...interface{}) int {
	return NewB(x)
}

var sink int

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewB([]interface{}{s})
	}
}

func BenchmarkC(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewC(s)
	}
}

The output:

BenchmarkA-4   	60167695	        49.68 ns/op	       8 B/op	       1 allocs/op
BenchmarkB-4   	629795134	         1.882 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	51212834	        43.10 ns/op	       8 B/op	       1 allocs/op
zigo101

zigo101 commented on Sep 30, 2021

@zigo101

The smallest case:

package main

import "testing"

func NewA(x ...interface{}) int {
	return 0
}

var sink int

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA([]interface{}{s}...)
	}
}

It looks the code inlining misleads the escape analysis, so that the arguments in the auto-constructed slice parameter are all allocated on heap.

{update]: if the value 99999 is changed to a value between [0, 255], then there will be no heap allocations.

tdakkota

tdakkota commented on Sep 30, 2021

@tdakkota
Contributor

if the value 99999 is changed to a value between [0, 255]

It seems a optimization introduced by CL 216401.

zigo101

zigo101 commented on Nov 1, 2021

@zigo101

It looks this has been fixed on tip.

leitzler

leitzler commented on Nov 15, 2022

@leitzler
Contributor

@dsnet this can be closed, right?

dsnet

dsnet commented on Nov 15, 2022

@dsnet
MemberAuthor

Correct. Confirmed that it is fixed.

locked and limited conversation to collaborators on Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.Performancecompiler/runtimeIssues related to the Go compiler and/or runtime.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @bradfitz@rogpeppe@leitzler@ianlancetaylor@dsnet

        Issue actions

          cmd/compile: stack allocate string and slice headers when passed through non-escaping interfaces · Issue #23676 · golang/go