Skip to content

Commit f7c378a

Browse files
committed
cmd/go: use critical path scheduling
DO NOT REVIEW This is a straight port of Dmitry's CL 13235046. TODO: * Understand better how it works. * Assign better weights. * Convince myself that it is an improvement. Early measurements are not yet compelling, although that could be because the weights are not well-chosen. Notes on weights from the issue: link > compile run test > link Fake instant actions << everything else cgo increases weights Fixes golang#8893. Change-Id: I8fba875a9a56aed132bd6b37b607e374f336728d
1 parent eced964 commit f7c378a

File tree

2 files changed

+54
-30
lines changed

2 files changed

+54
-30
lines changed

src/cmd/go/build.go

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,7 @@ type action struct {
532532

533533
f func(*builder, *action) error // the action itself (nil = no-op)
534534
ignoreFail bool // whether to run f even if dependencies fail
535+
weight int // estimated execution time (unitless)
535536

536537
// Generated files, directories.
537538
link bool // target is executable, not just package
@@ -789,31 +790,57 @@ func actionList(root *action) []*action {
789790
return all
790791
}
791792

792-
// do runs the action graph rooted at root.
793-
func (b *builder) do(root *action) {
794-
// Build list of all actions, assigning depth-first post-order priority.
795-
// The original implementation here was a true queue
796-
// (using a channel) but it had the effect of getting
797-
// distracted by low-level leaf actions to the detriment
798-
// of completing higher-level actions. The order of
799-
// work does not matter much to overall execution time,
800-
// but when running "go test std" it is nice to see each test
801-
// results as soon as possible. The priorities assigned
802-
// ensure that, all else being equal, the execution prefers
803-
// to do what it would have done first in a simple depth-first
804-
// dependency order traversal.
805-
all := actionList(root)
806-
for i, a := range all {
807-
a.priority = i
793+
func assignPriorities(all []*action) {
794+
for _, a := range all {
795+
a.weight = 1 // TODO: better weight estimation
796+
a.priority = -1
808797
}
809798

810-
b.readySema = make(chan bool, len(all))
799+
for assigned := 0; assigned < len(all); {
800+
for _, a := range all {
801+
if a.priority != -1 {
802+
continue
803+
}
804+
assigned++
805+
a.priority = 0
806+
for _, a1 := range a.triggers {
807+
if a1.priority == -1 {
808+
assigned--
809+
a.priority = -1
810+
break
811+
}
812+
if a.priority < a1.priority+1 {
813+
a.priority = a1.priority + a.weight
814+
}
815+
}
816+
}
817+
}
811818

812-
// Initialize per-action execution state.
819+
// TODO: DELETE
820+
// aq := actionQueue(all)
821+
// sort.Sort(&aq)
822+
// for _, a := range aq {
823+
// if a.p != nil {
824+
// fmt.Println("*** ", a.p.ImportPath, a.priority)
825+
// } else {
826+
// fmt.Println("--- ", a.objpkg, a.priority)
827+
// }
828+
// }
829+
}
830+
831+
// do runs the action graph rooted at root.
832+
func (b *builder) do(root *action) {
833+
all := actionList(root)
813834
for _, a := range all {
814835
for _, a1 := range a.deps {
815836
a1.triggers = append(a1.triggers, a)
816837
}
838+
}
839+
assignPriorities(all)
840+
841+
// Initialize per-action execution state.
842+
b.readySema = make(chan bool, len(all))
843+
for _, a := range all {
817844
a.pending = len(a.deps)
818845
if a.pending == 0 {
819846
b.ready.push(a)
@@ -2761,7 +2788,7 @@ type actionQueue []*action
27612788
// Implement heap.Interface
27622789
func (q *actionQueue) Len() int { return len(*q) }
27632790
func (q *actionQueue) Swap(i, j int) { (*q)[i], (*q)[j] = (*q)[j], (*q)[i] }
2764-
func (q *actionQueue) Less(i, j int) bool { return (*q)[i].priority < (*q)[j].priority }
2791+
func (q *actionQueue) Less(i, j int) bool { return (*q)[i].priority > (*q)[j].priority }
27652792
func (q *actionQueue) Push(x interface{}) { *q = append(*q, x.(*action)) }
27662793
func (q *actionQueue) Pop() interface{} {
27672794
n := len(*q) - 1

src/cmd/go/test.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,7 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
797797
computeStale(pmain)
798798

799799
if ptest != p {
800+
// Put the package into action cache with correct parameters.
800801
a := b.action(modeBuild, modeBuild, ptest)
801802
a.objdir = testDir + string(filepath.Separator) + "_obj_test" + string(filepath.Separator)
802803
a.objpkg = ptestObj
@@ -805,6 +806,7 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
805806
}
806807

807808
if pxtest != nil {
809+
// Put the package into action cache with correct parameters.
808810
a := b.action(modeBuild, modeBuild, pxtest)
809811
a.objdir = testDir + string(filepath.Separator) + "_obj_xtest" + string(filepath.Separator)
810812
a.objpkg = buildToolchain.pkgpath(testDir, pxtest)
@@ -874,14 +876,9 @@ func (b *builder) test(p *Package) (buildAction, runAction, printAction *action,
874876
p: p,
875877
ignoreFail: true,
876878
}
877-
cleanAction := &action{
878-
f: (*builder).cleanTest,
879-
deps: []*action{runAction},
880-
p: p,
881-
}
882879
printAction = &action{
883880
f: (*builder).printTest,
884-
deps: []*action{cleanAction},
881+
deps: []*action{runAction},
885882
p: p,
886883
}
887884
}
@@ -978,6 +975,8 @@ func declareCoverVars(importPath string, files ...string) map[string]*CoverVar {
978975

979976
// runTest is the action for running a test binary.
980977
func (b *builder) runTest(a *action) error {
978+
defer b.cleanTest(a)
979+
981980
args := stringList(findExecCmd(), a.deps[0].target, testArgs)
982981
a.testOutput = new(bytes.Buffer)
983982

@@ -1105,20 +1104,18 @@ func coveragePercentage(out []byte) string {
11051104
}
11061105

11071106
// cleanTest is the action for cleaning up after a test.
1108-
func (b *builder) cleanTest(a *action) error {
1107+
func (b *builder) cleanTest(a *action) {
11091108
if buildWork {
1110-
return nil
1109+
return
11111110
}
11121111
run := a.deps[0]
11131112
testDir := filepath.Join(b.work, filepath.FromSlash(run.p.ImportPath+"/_test"))
11141113
os.RemoveAll(testDir)
1115-
return nil
11161114
}
11171115

11181116
// printTest is the action for printing a test result.
11191117
func (b *builder) printTest(a *action) error {
1120-
clean := a.deps[0]
1121-
run := clean.deps[0]
1118+
run := a.deps[0]
11221119
os.Stdout.Write(run.testOutput.Bytes())
11231120
run.testOutput = nil
11241121
return nil

0 commit comments

Comments
 (0)