google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.32k stars 212 forks source link

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

Closed adonovan closed 2 years ago

adonovan commented 2 years ago

The Int optimization uses a 4GB virtual address allocation to represent 32-bit values as addresses 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, a linux-only test.

Also, 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.)

Fixes #394

adonovan commented 2 years ago

Ping @brandjon...

aarzilli commented 2 years ago

Ping?

adonovan commented 2 years ago

Seems reasonable. Would be good to have a test, though (e.g. set RLIMIT_AS, see if int tests still pass).

Done, though it's a pain because it requires fork+execing a child process, and it requires ulimit -v which is not available on MacOS or Windows.

adonovan commented 2 years ago

@tetromino, PTAL at the changes to the build tags, go.mod/sum files, Actions workflow config, and of course the test.