starlark: fall back to slow Ints if mmap fails (google#398)

The Int optimization uses a 4GB virtual address allocation to represent
32-bit values as addressed to avoid allocation. However, some environments
have limited address space.

This change permits the mmap to fail, and in that case it prints a warning
and falls back to always allocating a big.Int, even for small numbers.
Each access to an Int must check the smallints global to see whether the
optimization is active.  The extra dynamic check doesn't cost much:

$ go test -run=nope -bench=BenchmarkStarlark -count=3 ./starlark    # > before, after
$ go install golang.org/x/perf/cmd/benchstat@latest
$ ~/go/bin/benchstat  before after
name                                  old time/op  new time/op   delta
Starlark/bench_bigint-16               206µs ± 0%    212µs ± 0%   ~     (p=0.100 n=3+3)
Starlark/bench_builtin_method-16       283µs ± 1%    294µs ± 9%   ~     (p=1.000 n=3+3)
Starlark/bench_calling-16              280µs ± 1%    290µs ± 1%   ~     (p=0.100 n=3+3)
Starlark/bench_gauss-16               9.30ms ± 5%   9.55ms ± 2%   ~     (p=0.700 n=3+3)
Starlark/bench_int-16                 53.7µs ± 1%   59.6µs ± 1%   ~     (p=0.100 n=3+3)
Starlark/bench_mix-16                 99.3µs ± 1%  106.6µs ±10%   ~     (p=0.100 n=3+3)
Starlark/bench_range_construction-16   238ns ± 1%    244ns ± 2%   ~     (p=0.100 n=3+3)
Starlark/bench_range_iteration-16     5.44µs ± 2%   5.67µs ± 1%   ~     (p=0.100 n=3+3)

Also:
- Add a linux-only test.
- Simplify the build tags now that the 64-bit POSIX exceptions (iOS, openbsd)
  are handled dynamically. As a side effect, M1-based Macs should get the optimization
  for the first time. (Requires updating sys module.)
- Update Actions tests to go1.18, 1.19, and drop 1.16 and 1.17.

Fixes google#394

Co-authored-by: Alan Donovan <[email protected]>

.github/workflows/tests.yml

@@ -5,7 +5,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        go-version: [1.16.x, 1.17.x]
+        go-version: [1.18.x, 1.19.x]
     runs-on: ${{ matrix.os }}
     steps:
       - name: Install Go

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e
 	github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect
 	github.com/google/go-cmp v0.5.1 // indirect
-	golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f
+	golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	google.golang.org/protobuf v1.25.0
 )

go.sum

@@ -43,8 +43,8 @@ golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f h1:+Nyd8tzPX9R7BWHguqsrbFdRx3WQ/1ib8I44HXV5yTA=
-golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab h1:2QkjZIsXupsJbJIdSjjUOgWK3aEtzyuh2mPt3l/CkeU=
+golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=

starlark/bench_test.go

@@ -16,7 +16,7 @@ import (
 	"go.starlark.net/starlarktest"
 )
 
-func Benchmark(b *testing.B) {
+func BenchmarkStarlark(b *testing.B) {
 	defer setOptions("")
 
 	testdata := starlarktest.DataFile("starlark", ".")

starlark/int.go

@@ -46,13 +46,18 @@ func MakeUint64(x uint64) Int {
 // MakeBigInt returns a Starlark int for the specified big.Int.
 // The new Int value will contain a copy of x. The caller is safe to modify x.
 func MakeBigInt(x *big.Int) Int {
-	if n := x.BitLen(); n < 32 || n == 32 && x.Int64() == math.MinInt32 {
+	if isSmall(x) {
 		return makeSmallInt(x.Int64())
 	}
 	z := new(big.Int).Set(x)
 	return makeBigInt(z)
 }
 
+func isSmall(x *big.Int) bool {
+	n := x.BitLen()
+	return n < 32 || n == 32 && x.Int64() == math.MinInt32
+}
+
 var (
 	zero, one = makeSmallInt(0), makeSmallInt(1)
 	oneBig    = big.NewInt(1)

starlark/int_generic.go

@@ -1,4 +1,5 @@
-//+build !linux,!darwin,!dragonfly,!freebsd,!netbsd,!solaris darwin,arm64 !amd64,!arm64,!mips64x,!ppc64x,!loong64
+//go:build (!linux && !darwin && !dragonfly && !freebsd && !netbsd && !solaris) || (!amd64 && !arm64 && !mips64x && !ppc64x && !loong64)
+// +build !linux,!darwin,!dragonfly,!freebsd,!netbsd,!solaris !amd64,!arm64,!mips64x,!ppc64x,!loong64
 
 package starlark
 

starlark/int_posix64.go

@@ -1,5 +1,6 @@
-//+build linux darwin dragonfly freebsd netbsd solaris
-//+build amd64 arm64,!darwin mips64x ppc64x loong64
+//go:build (linux || darwin || dragonfly || freebsd || netbsd || solaris) && (amd64 || arm64 || mips64x || ppc64x || loong64)
+// +build linux darwin dragonfly freebsd netbsd solaris
+// +build amd64 arm64 mips64x ppc64x loong64
 
 package starlark
 
@@ -10,16 +11,12 @@ package starlark
 // values be represented as an unsafe.Pointer, so that Int-to-Value
 // interface conversion need not allocate.
 
-// Although iOS (arm64,darwin) claims to be a POSIX-compliant,
-// it limits each process to about 700MB of virtual address space,
-// which defeats the optimization.
-//
-// TODO(golang.org/issue/38485): darwin,arm64 may refer to macOS in the future.
-// Update this when there are distinct GOOS values for macOS, iOS, and other Apple
-// operating systems on arm64.
-//
-// This optimization is disabled on OpenBSD, because its default
-// ulimit for virtual memory is a measly GB or so.
+// Although iOS (which, like macOS, appears as darwin/arm64) is
+// POSIX-compliant, it limits each process to about 700MB of virtual
+// address space, which defeats the optimization.  Similarly,
+// OpenBSD's default ulimit for virtual memory is a measly GB or so.
+// On both those platforms the attempted optimization will fail and
+// fall back to the slow implementation.
 
 // An alternative approach to this optimization would be to embed the
 // int32 values in pointers using odd values, which can be distinguished
@@ -41,22 +38,36 @@ import (
 // so that Int-to-Value conversions need not allocate.
 //
 // The pointer is either a *big.Int, if the value is big, or a pointer into a
-// reserved portion of the address space (smallints), if the value is small.
+// reserved portion of the address space (smallints), if the value is small
+// and the address space allocation succeeded.
 //
 // See int_generic.go for the basic representation concepts.
 type intImpl unsafe.Pointer
 
 // get returns the (small, big) arms of the union.
 func (i Int) get() (int64, *big.Int) {
-	ptr := uintptr(i.impl)
-	if ptr >= smallints && ptr < smallints+1<<32 {
+	if smallints == 0 {
+		// optimization disabled
+		if x := (*big.Int)(i.impl); isSmall(x) {
+			return x.Int64(), nil
+		} else {
+			return 0, x
+		}
+	}
+
+	if ptr := uintptr(i.impl); ptr >= smallints && ptr < smallints+1<<32 {
 		return math.MinInt32 + int64(ptr-smallints), nil
 	}
 	return 0, (*big.Int)(i.impl)
 }
 
 // Precondition: math.MinInt32 <= x && x <= math.MaxInt32
 func makeSmallInt(x int64) Int {
+	if smallints == 0 {
+		// optimization disabled
+		return Int{intImpl(big.NewInt(x))}
+	}
+
 	return Int{intImpl(uintptr(x-math.MinInt32) + smallints)}
 }
 
@@ -66,12 +77,15 @@ func makeBigInt(x *big.Int) Int { return Int{intImpl(x)} }
 // smallints is the base address of a 2^32 byte memory region.
 // Pointers to addresses in this region represent int32 values.
 // We assume smallints is not at the very top of the address space.
+//
+// Zero means the optimization is disabled and all Ints allocate a big.Int.
 var smallints = reserveAddresses(1 << 32)
 
 func reserveAddresses(len int) uintptr {
 	b, err := unix.Mmap(-1, 0, len, unix.PROT_READ, unix.MAP_PRIVATE|unix.MAP_ANON)
 	if err != nil {
-		log.Fatalf("mmap: %v", err)
+		log.Printf("Starlark failed to allocate 4GB address space: %v. Integer performance may suffer.", err)
+		return 0 // optimization disabled
 	}
 	return uintptr(unsafe.Pointer(&b[0]))
 }

starlark/int_test.go

@@ -5,9 +5,15 @@
 package starlark
 
 import (
+	"flag"
 	"fmt"
+	"log"
 	"math"
 	"math/big"
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
 	"testing"
 )
 
@@ -100,3 +106,50 @@ func TestImmutabilityBigInt(t *testing.T) {
 		}
 	}
 }
+
+// TestIntFallback creates a small Int value in a child process with
+// limited address space to ensure that it still works, but prints a warning.
+func TestIntFallback(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skipf("test disabled on this platform (requires ulimit -v)")
+	}
+	exe, err := os.Executable()
+	if err != nil {
+		t.Fatalf("can't find file name of executable: %v", err)
+	}
+	// ulimit -v limits the address space in KB. Not portable.
+	// 1GB is enough for the Go runtime but not for the optimization.
+	cmd := exec.Command("/bin/sh", "-c", fmt.Sprintf("ulimit -v 1000000 && %q --entry=intfallback", exe))
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("intfallback subcommand failed: %v\n%s", err, out)
+	}
+
+	// Check the warning was printed.
+	if !strings.Contains(string(out), "Integer performance may suffer") {
+		t.Errorf("expected warning was not printed. Output=<<%s>>", out)
+	}
+}
+
+// intfallback is called in a child process with limited address space.
+func intfallback() {
+	const want = 123
+	if got, _ := MakeBigInt(big.NewInt(want)).Int64(); got != want {
+		log.Fatalf("intfallback: got %d, want %d", got, want)
+	}
+}
+
+// The --entry flag invokes an alternate entry point, for use in subprocess tests.
+func TestMain(m *testing.M) {
+	var entry string
+	flag.StringVar(&entry, "entry", "", "child process entry-point")
+	flag.Parse()
+	switch entry {
+	case "":
+		os.Exit(m.Run()) // normal case
+	case "intfallback":
+		intfallback()
+	default:
+		log.Fatalf("unknown entry point: %s", entry)
+	}
+}