From a2d0d6b08b1455e2d047756f036b3171ee0f51db Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Sat, 1 Jun 2019 16:05:16 -0400 Subject: [PATCH] server: reimplement growStack hack in assembly 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. --- pkg/server/node.go | 25 ++++--------------------- pkg/util/growstack/growstack.go | 24 ++++++++++++++++++++++++ pkg/util/growstack/growstack.s | 19 +++++++++++++++++++ 3 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 pkg/util/growstack/growstack.go create mode 100644 pkg/util/growstack/growstack.s diff --git a/pkg/server/node.go b/pkg/server/node.go index 8fa8628ba60f..48ba13233fe9 100644 --- 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 new file mode 100644 index 000000000000..bb62b044ab0f --- /dev/null +++ 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 new file mode 100644 index 000000000000..3361fa2a8318 --- /dev/null +++ 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