{"action":"created","issue":{"url":"https://api.github.com/repos/golang/go/issues/20070","repository_url":"https://api.github.com/repos/golang/go","labels_url":"https://api.github.com/repos/golang/go/issues/20070/labels{/name}","comments_url":"https://api.github.com/repos/golang/go/issues/20070/comments","events_url":"https://api.github.com/repos/golang/go/issues/20070/events","html_url":"https://github.com/golang/go/issues/20070","id":223473727,"number":20070,"title":"cmd/compile: read and process import data lazily","user":{"login":"rsc","id":104030,"avatar_url":"https://avatars2.githubusercontent.com/u/104030?v=3","gravatar_id":"","url":"https://api.github.com/users/rsc","html_url":"https://github.com/rsc","followers_url":"https://api.github.com/users/rsc/followers","following_url":"https://api.github.com/users/rsc/following{/other_user}","gists_url":"https://api.github.com/users/rsc/gists{/gist_id}","starred_url":"https://api.github.com/users/rsc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rsc/subscriptions","organizations_url":"https://api.github.com/users/rsc/orgs","repos_url":"https://api.github.com/users/rsc/repos","events_url":"https://api.github.com/users/rsc/events{/privacy}","received_events_url":"https://api.github.com/users/rsc/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/golang/go/milestones/55","html_url":"https://github.com/golang/go/milestone/55","labels_url":"https://api.github.com/repos/golang/go/milestones/55/labels","id":2473073,"number":55,"title":"Go1.10Early","description":"","creator":{"login":"rsc","id":104030,"avatar_url":"https://avatars2.githubusercontent.com/u/104030?v=3","gravatar_id":"","url":"https://api.github.com/users/rsc","html_url":"https://github.com/rsc","followers_url":"https://api.github.com/users/rsc/followers","following_url":"https://api.github.com/users/rsc/following{/other_user}","gists_url":"https://api.github.com/users/rsc/gists{/gist_id}","starred_url":"https://api.github.com/users/rsc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rsc/subscriptions","organizations_url":"https://api.github.com/users/rsc/orgs","repos_url":"https://api.github.com/users/rsc/repos","events_url":"https://api.github.com/users/rsc/events{/privacy}","received_events_url":"https://api.github.com/users/rsc/received_events","type":"User","site_admin":false},"open_issues":1,"closed_issues":0,"state":"open","created_at":"2017-04-21T19:22:10Z","updated_at":"2017-04-21T19:22:36Z","due_on":"2017-10-31T07:00:00Z","closed_at":null},"comments":3,"created_at":"2017-04-21T19:22:36Z","updated_at":"2017-04-21T22:09:35Z","closed_at":null,"body":"As an experiment, I changed the compiler to report the number of packages, types, and functions built up as a result of processing imports for a given package:\r\n\r\n```\r\n$ git diff \r\ndiff --git a/src/cmd/compile/internal/gc/bimport.go b/src/cmd/compile/internal/gc/bimport.go\r\nindex ea9c02dea8..4fee3eee41 100644\r\n--- a/src/cmd/compile/internal/gc/bimport.go\r\n+++ b/src/cmd/compile/internal/gc/bimport.go\r\n@@ -15,6 +15,7 @@ import (\r\n \t\"encoding/binary\"\r\n \t\"fmt\"\r\n \t\"math/big\"\r\n+\t\"os\"\r\n \t\"strconv\"\r\n \t\"strings\"\r\n )\r\n@@ -220,6 +221,10 @@ func Import(imp *types.Pkg, in *bufio.Reader) {\r\n \r\n \tp.verifyTypes()\r\n \r\n+\tif os.Getenv(\"IMPORTDEBUG\") != \"\" {\r\n+\t\tfmt.Fprintf(os.Stderr, \"import %v: %d pkg + %d type + %d func\\n\", imp.Path, len(p.pkgList), len(p.typList), len(p.funcList))\r\n+\t}\r\n+\r\n \t// --- end of export data ---\r\n \r\n \ttypecheckok = tcok\r\n$ \r\n```\r\n\r\nThis program has no imports, so no overhead:\r\n\r\n```\r\n$ cat x.go\r\npackage main\r\n\r\nfunc main() {\r\n\tprintln(\"hello world\")\r\n}\r\n$ IMPORTDEBUG=1 go build x.go\r\n$\r\n```\r\n\r\nThis program imports fmt, which turns out to mean processing 38 types and 20 functions, only one of which is needed:\r\n\r\n```\r\n$ cat x.go\r\npackage main\r\n\r\nimport \"fmt\"\r\n\r\nfunc main() {\r\n\tfmt.Println(\"hello world\\n\")\r\n}\r\n$ IMPORTDEBUG=1 go build x.go\r\n# command-line-arguments\r\nimport fmt: 2 pkg + 38 type + 20 func\r\n$\r\n```\r\n\r\nThis program imports net/http, which turns out to mean processing 231 types and 857 functions, only one of which is needed:\r\n\r\n```\r\n$ cat x.go\r\npackage main\r\n\r\nimport \"net/http\"\r\n\r\nfunc main() {\r\n\thttp.ListenAndServe(\":8080\", nil)\r\n}\r\n$ IMPORTDEBUG=1 go build x.go\r\n# command-line-arguments\r\nimport net/http: 29 pkg + 231 type + 857 func\r\n$\r\n```\r\n\r\nThe overhead here is more than 100X. Many real-world packages are even bigger than the ones in the standard library.\r\n\r\nIn a typical source file or package, especially during an incremental build, I suspect the compiler spends significantly more type loading and type-checking all these unnecessary imported definitions than it does the necessary one and the actual source file.\r\n\r\nNow that we have a clean encapsulation of import/export format handling, it seems like the next step should be to make the import/export format indexed in some way, so that it can be consulted on demand during the compilation rather than loaded entirely up front. That is, when you import \"fmt\" you'd open the export data for fmt, and then when you get to fmt.Println you'd pull in just what you need to understand fmt.Println (in this case, likely just the function type).\r\n\r\nSimilarly, for the HTTP server, invoking http.ListenAndServe would mean pulling in the function type and possibly then also learning about http.Handler, since that is the type of the second argument. That might in turn require learning about some more types, since Handler's ServeHTTP takes an http.ResponseWriter and a \\*http.Request. The latter is a struct with many other types, and it might require loading some large number of other types. Ideally this could be cut off through more lazy loading (none of this is required to type check that the argument\r\nnil is in fact a \\*http.Request). Even if not, though, the total number of types and functions\r\nshould still be smaller than before.\r\n\r\nThe problem will only get worse as packages grow. For protobuf-generated code, where the export data summarizes essentially the entirety of all imported packages recursively, not loading this information into the compiler until it is needed would be a big win.\r\n\r\nEven in the main repo, there are indications this could be a big win. Consider a full build of cmd/go (just to take a decent-sized program) and its dependencies, compiled with inlining completely disabled:\r\n\r\n```\r\n$ for i in $(seq 5); do 9 time go build -o go.exe -gcflags -l=1 -a cmd/go; sleep 5; done\r\n14.56u 2.70s 6.65r \t go build -o go.exe -gcflags ...\r\n14.69u 2.71s 6.70r \t go build -o go.exe -gcflags ...\r\n14.84u 2.74s 6.83r \t go build -o go.exe -gcflags ...\r\n14.91u 2.74s 6.88r \t go build -o go.exe -gcflags ...\r\n14.76u 2.72s 6.74r \t go build -o go.exe -gcflags ...\r\n$ \r\n```\r\n\r\nHere's the same full build with our current default inlining:\r\n\r\n```\r\n$ for i in $(seq 5); do 9 time go build -o go.exe -gcflags -l=0 -a cmd/go; sleep 5; done\r\n16.51u 2.80s 7.31r \t go build -o go.exe -gcflags ...\r\n16.79u 2.86s 7.27r \t go build -o go.exe -gcflags ...\r\n16.97u 2.88s 7.37r \t go build -o go.exe -gcflags ...\r\n17.03u 2.88s 7.41r \t go build -o go.exe -gcflags ...\r\n17.11u 2.86s 7.38r \t go build -o go.exe -gcflags ...\r\n$ \r\n```\r\n\r\nAnd here's the same full build with the more aggressive mid-stack inlining:\r\n\r\n```\r\n$ for i in $(seq 5); do 9 time go build -o go.exe -gcflags -l=4 -a cmd/go; sleep 5; done\r\n24.09u 3.26s 9.53r \t go build -o go.exe -gcflags ...\r\n24.08u 3.25s 9.28r \t go build -o go.exe -gcflags ...\r\n24.59u 3.30s 9.61r \t go build -o go.exe -gcflags ...\r\n24.39u 3.26s 9.52r \t go build -o go.exe -gcflags ...\r\n24.63u 3.28s 9.52r \t go build -o go.exe -gcflags ...\r\n$ \r\n```\r\n\r\nThe difference here is that more inlining means more function bodies that must be imported and type-checked. My guess is that, for any single package being compiled, nearly all of the function bodies included in export data are irrelevant (but which ones are irrelevant changes depending\r\non who is importing the package, of course). Under the assumption that on-demand processing of the function bodies would make `-l=0` (inlining on) behave more like `-l=1` (inlining off), that would reduce build times by about 10% (6.65s vs 7.27s). It would also keep the more aggressive inlining from increasing build times by 25% (9.28s vs 7.27s).\r\n\r\nI'm assuming here that the increase in time is due to importing the new data and not writing it out. I haven't checked that, but the export data is typically read back many times but only written out once (or zero times) during a given build, so I think that's a reasonable assumption.\r\n\r\nIt's obviously too late for Go 1.9, but it seems like this would be an effective way to speed up the compiler in the Go 1.10 cycle.\r\n\r\n/cc @aclements @griesemer @mdempsky"},"comment":{"url":"https://api.github.com/repos/golang/go/issues/comments/296316999","html_url":"https://github.com/golang/go/issues/20070#issuecomment-296316999","issue_url":"https://api.github.com/repos/golang/go/issues/20070","id":296316999,"user":{"login":"josharian","id":67496,"avatar_url":"https://avatars1.githubusercontent.com/u/67496?v=3","gravatar_id":"","url":"https://api.github.com/users/josharian","html_url":"https://github.com/josharian","followers_url":"https://api.github.com/users/josharian/followers","following_url":"https://api.github.com/users/josharian/following{/other_user}","gists_url":"https://api.github.com/users/josharian/gists{/gist_id}","starred_url":"https://api.github.com/users/josharian/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/josharian/subscriptions","organizations_url":"https://api.github.com/users/josharian/orgs","repos_url":"https://api.github.com/users/josharian/repos","events_url":"https://api.github.com/users/josharian/events{/privacy}","received_events_url":"https://api.github.com/users/josharian/received_events","type":"User","site_admin":false},"created_at":"2017-04-21T22:09:35Z","updated_at":"2017-04-21T22:09:35Z","body":"> The difference here is that more inlining means more function bodies that must be imported and type-checked.\r\n\r\nTrue, but the importing and type-checking is not be the whole story. Inlining also means that functions get larger, which means more nodes, more time in the backend, more progs, etc. I'd wager that those effects are also non-trivial.\r\n\r\nI still think this is a good idea, I'm just not sure that the inlining data is clear evidence for it, or that it will necessarily make mid-stack inlining much cheaper, compilation-wise.\r\n"}}
{"actor":{"display_login":"josharian"}}