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: reuse temporaries introduced by compiler #21557

Closed
randall77 opened this issue Aug 22, 2017 · 3 comments
Closed

cmd/compile: reuse temporaries introduced by compiler #21557

randall77 opened this issue Aug 22, 2017 · 3 comments

Comments

@randall77
Copy link
Contributor

@randall77 randall77 commented Aug 22, 2017

package main

type T struct {
	p *int
	i int
}

func f(m map[T]int) {
	m[T{nil, 1}] = 1
	m[T{nil, 2}] = 2
}

If you look at the disassembly of f, we use an autotmp to pass the key to mapassign. Each call to mapassign uses its own temporary. We should reuse these temporaries - they are live only for the duration of the mapassign call.

This situation gets particularly painful for large map initializations in init() functions. There could be thousands of mapassign calls, and each one gets its own temporary. That means really large stack frames and correspondingly large live pointer maps. In fact the live pointer map size is quadratic in source size, as it is proportional to both the number of temporaries and the number of safe points.

There are other places we use temporaries like this, mostly introduced in order.go. We should reuse temps for as many as we can.

Note: I use the weird key in the example to avoid the _fast version for int32, int64, and string. _fast versions don't use an autotmp.

Related: #19751

@josharian @mdempsky @martisch

@randall77 randall77 added this to the Go1.10 milestone Aug 22, 2017
@randall77 randall77 added the ToolSpeed label Aug 22, 2017
@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Aug 23, 2017

I tried this when I was working on #19751 and found that it didn't help all that much; maybe I did it wrong though. It was also a pain to implement, due to how walk and sinit ping-pong back and forth--there was no good place to put state, but again, I'd be delighted to have been wrong about this.

@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 29, 2017
@mdempsky

This comment has been minimized.

Copy link
Member

@mdempsky mdempsky commented Nov 29, 2017

Still desirable, but too late for 1.10.

@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 19, 2018
@randall77

This comment has been minimized.

Copy link
Contributor Author

@randall77 randall77 commented Nov 17, 2018

This was fixed with CL 140301.

@randall77 randall77 closed this Nov 17, 2018
@golang golang locked and limited conversation to collaborators Nov 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.