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.

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

cmd/compile: lay out global errors.New results in static data #30820

Open
rsc opened this Issue Mar 14, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@rsc
Copy link
Contributor

rsc commented Mar 14, 2019

A significant fraction of init time in some programs
(notably the go command but surely others) is spent
executing lines like:

var ErrMyError = errors.New("my error")

This is such a widespread use that it may well be
worth recognizing in the compiler and turning into
static data, so that it can be laid out in the data section,
discarded by the linker as appropriate, and have no
link-time overhead at all.

The specific pattern would be exactly the above
(var foo = errors.New(constString)) and it would turn into

var foo = error(&errors.errorString{s: constString})

This would require that the compiler be able to lay out an
implicit interface conversion in the data section as well.
If that can't be done in general, it would suffice to special
case the "errors.errorString to error" conversion.

If this sounds like fun to someone familiar with the
compiler, please have a look. Thanks.

@rsc rsc added this to the Go1.13 milestone Mar 14, 2019

@randall77

This comment has been minimized.

Copy link
Contributor

randall77 commented Mar 14, 2019

The compiler already does the latter part of what you described. Starting with:

type errorString struct {
	s string
}

func (e *errorString) Error() string {
	return e.s
}

var foo = error(&errorString{s: "foobar"})

The compiler generates:

"".foo SDATA size=16
	0x0000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	rel 0+8 t=1 go.itab.*"".errorString,error+0
	rel 8+8 t=1 ""..stmp_0+0
""..stmp_0 SDATA size=16
	0x0000 00 00 00 00 00 00 00 00 06 00 00 00 00 00 00 00  ................
	rel 0+8 t=1 go.string."foobar"+0
go.string."foobar" SRODATA dupok size=6
	0x0000 66 6f 6f 62 61 72                                foobar

@ALTree ALTree added the Performance label Mar 14, 2019

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Mar 14, 2019

This would be a nice follow-up step to #30468.

/cc @bcmills @neild @mpvl

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Mar 14, 2019

I wonder, would there be a plan to do the same for simple global uses of fmt.Errorf like var ErrFoo = fmt.Errorf("foo error")?

Not that fmt.Errorf is necessarily better for global errors, but i f we made errors.New comparatively inexpensive, it could lead to a recommended practice to never use fmt.Errorf for globals.

Which is maybe fine, but ideally we don't want developers to have to remember that distinction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.