Merge cockroachdb#37967

37967: server: reimplement growStack hack in assembly r=nvanbenschoten a=petermattis The go1.12 compiler breaks the Go version of the growStack hack (it optimizes the 16kb stack variable out of existence). Reimplement growStack in assembly which will be same from the machinations of the Go compiler. Release note (performance improvement): Fix a significant performance regression when building CockroachDB with go1.12. Co-authored-by: Peter Mattis <[email protected]>
nvanbenschoten · Jun 3, 2019 · 5a17b3d · 5a17b3d
2 parents b0f95de + a2d0d6b
commit 5a17b3d
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 21 deletions.
diff --git a/pkg/server/node.go b/pkg/server/node.go
@@ -37,6 +37,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/engine"
 	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/growstack"
 	"github.com/cockroachdb/cockroach/pkg/util/grpcutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -936,7 +937,7 @@ func (n *Node) batchInternal(
 func (n *Node) Batch(
 	ctx context.Context, args *roachpb.BatchRequest,
 ) (*roachpb.BatchResponse, error) {
-	growStack()
+	growstack.Grow()
 
 	// NB: Node.Batch is called directly for "local" calls. We don't want to
 	// carry the associated log tags forward as doing so makes adding additional
@@ -1023,6 +1024,8 @@ func (n *Node) setupSpanForIncomingRPC(
 func (n *Node) RangeFeed(
 	args *roachpb.RangeFeedRequest, stream roachpb.Internal_RangeFeedServer,
 ) error {
+	growstack.Grow()
+
 	pErr := n.stores.RangeFeed(args, stream)
 	if pErr != nil {
 		var event roachpb.RangeFeedEvent
@@ -1033,23 +1036,3 @@ func (n *Node) RangeFeed(
 	}
 	return nil
 }
-
-var growStackGlobal = false
-
-//go:noinline
-func growStack() {
-	// Goroutine stacks currently start at 2 KB in size. The code paths through
-	// the storage package often need a stack that is 32 KB in size. The stack
-	// growth is mildly expensive making it useful to trick the runtime into
-	// growing the stack early. Since goroutine stacks grow in multiples of 2 and
-	// start at 2 KB in size, by placing a 16 KB object on the stack early in the
-	// lifetime of a goroutine we force the runtime to use a 32 KB stack for the
-	// goroutine.
-	var buf [16 << 10] /* 16 KB */ byte
-	if growStackGlobal {
-		// Make sure the compiler doesn't optimize away buf.
-		for i := range buf {
-			buf[i] = byte(i)
-		}
-	}
-}
diff --git a/pkg/util/growstack/growstack.go b/pkg/util/growstack/growstack.go
@@ -0,0 +1,24 @@
+// Copyright 2019 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package growstack
+
+// Grow grows the goroutine stack by 16 KB. Goroutine stacks currently start
+// at 2 KB in size. The code paths through the storage package often need a
+// stack that is 32 KB in size. The stack growth is mildly expensive making it
+// useful to trick the runtime into growing the stack early. Since goroutine
+// stacks grow in multiples of 2 and start at 2 KB in size, by placing a 16 KB
+// object on the stack early in the lifetime of a goroutine we force the
+// runtime to use a 32 KB stack for the goroutine.
+func Grow()
diff --git a/pkg/util/growstack/growstack.s b/pkg/util/growstack/growstack.s
@@ -0,0 +1,19 @@
+// Copyright 2019 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+#include "funcdata.h"
+
+TEXT ·Grow(SB),0,$16384-0
+        NO_LOCAL_POINTERS
+        RET