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

Jump to bottom

cmd/compile: investigate SSA stack size regressions #15597

Closed
josharian opened this issue May 8, 2016 · 1 comment
Closed

cmd/compile: investigate SSA stack size regressions #15597

josharian opened this issue May 8, 2016 · 1 comment
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@josharian
Copy link
Contributor

For the most part, SSA generates smaller stacks than the old backend, but not always. This issue is to (a) document an easy way to find such regressions and (b) serve as a reminder to investigate some of them to see whether there are reasonable improvements available.

First, apply this diff, so that -S prints just package, function, and stack size:

diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go
index a1fdee6..ea13412 100644
--- a/src/cmd/internal/obj/objfile.go
+++ b/src/cmd/internal/obj/objfile.go
@@ -114,6 +114,7 @@ import (
        "log"
        "path/filepath"
        "sort"
+       "strings"
 )

 // The Go and C compilers, and the assembler, call writeobj to write
@@ -310,6 +311,11 @@ func (w *objWriter) writeRefs(s *LSym) {

 func (w *objWriter) writeSymDebug(s *LSym) {
        ctxt := w.ctxt
+       if s.Type != STEXT {
+               return
+       }
+       fmt.Fprintf(ctxt.Bso, "STACK %s.%s %d\n", ctxt.Pathname, strings.TrimPrefix(s.Name, `"".`), s.Locals)
+       return
        fmt.Fprintf(ctxt.Bso, "%s ", s.Name)
        if s.Version != 0 {
                fmt.Fprintf(ctxt.Bso, "v=%d ", s.Version)

Then

$ go build -a -gcflags="-S" std 2>&1 | sort > ssa.stacksize
$ go build -a -gcflags="-S -ssa=0" std 2>&1 | sort > nossa.stacksize
$ diff --suppress-common-lines --width 10000 --side-by-side nossa.stacksize ssa.stacksize | awk '{if ($2!=$6) {next} if(($7==0) && ($3 ==0)) {next} if ($7<=$3) {next} print $3/$7" "$2" "$3" -> "$7}' | sort -n

This is using OS X's diff. This prints four columns: old stack size / new stack size, package+function, old stack size, new stack size.

A few sample lines, pulled from a few places in the output:

0 crypto/cipher.fastXORBytes 0 -> 8
0 image.yCbCrSize 0 -> 8
0 math/big.shlVU_g 0 -> 8
0 math.qzero 0 -> 96
0 testing.(*B).add 0 -> 136
0.0248963 image/color/palette.init 48 -> 1928
0.0444444 vendor/golang.org/x/net/http2/hpack.init 112 -> 2520
0.75 sort.doPivot 96 -> 128

Full output at commit e6ec820 is at https://gist.github.com/josharian/9919226c603e9c90a367738962241a06.

cc @randall77 @dr2chase
@josharian josharian added this to the Go1.8 milestone May 8, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 11, 2016
@rsc rsc modified the milestones: Go1.9Early, Go1.8 Oct 21, 2016
@rsc rsc added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Oct 21, 2016
@bradfitz bradfitz modified the milestones: Go1.9Early, Go1.10Early May 3, 2017
@bradfitz bradfitz added early-in-cycle A change that should be done early in the 3 month dev cycle. and removed early-in-cycle A change that should be done early in the 3 month dev cycle. labels Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017
@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 30, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 May 18, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone May 31, 2018
@randall77
Copy link
Contributor

I'm going to close this, it's not really actionable any more.
It would still be nice to shrink stack frames as much as we can. But this method of investigation no longer works :(

@golang golang locked and limited conversation to collaborators Nov 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
8 participants
@bradfitz @mdempsky @josharian @rsc @quentinmit @ianlancetaylor @randall77 @gopherbot