always use the compiler's -dwarf=false flag (#96)
First, our original append line was completely ineffective; we never
used that "flags" slice again. Second, we only attempted to use the flag
when we obfuscated a package.
In fact, we never care about debugging information here, so for any
package we compile, we can add "-dwarf=false". At the moment, we compile
all packages, even if they aren't to be obfuscated, due to the lack of
access to the build cache.
As such, we save a significant amount of work. The numbers below were
obtained on a quiet machine with "go test -bench=. -benchtime=10x", six
times before and after the change.
name old time/op new time/op delta
Build-8 2.06s ± 4% 1.87s ± 2% -9.21% (p=0.002 n=6+6)
name old sys-time/op new sys-time/op delta
Build-8 1.51s ± 2% 1.46s ± 1% -3.12% (p=0.004 n=6+5)
name old user-time/op new user-time/op delta
Build-8 11.9s ± 2% 10.8s ± 1% -8.71% (p=0.002 n=6+6)
While at it, only do CI builds on pushes and PRs to the master branch,
so that my PRs created from the same repo don't trigger duplicate
builds.
4 years ago
|
|
|
on:
|
|
|
|
|
push:
|
|
|
|
|
branches:
|
|
|
|
|
- master
|
|
|
|
|
- ci-test
|
always use the compiler's -dwarf=false flag (#96)
First, our original append line was completely ineffective; we never
used that "flags" slice again. Second, we only attempted to use the flag
when we obfuscated a package.
In fact, we never care about debugging information here, so for any
package we compile, we can add "-dwarf=false". At the moment, we compile
all packages, even if they aren't to be obfuscated, due to the lack of
access to the build cache.
As such, we save a significant amount of work. The numbers below were
obtained on a quiet machine with "go test -bench=. -benchtime=10x", six
times before and after the change.
name old time/op new time/op delta
Build-8 2.06s ± 4% 1.87s ± 2% -9.21% (p=0.002 n=6+6)
name old sys-time/op new sys-time/op delta
Build-8 1.51s ± 2% 1.46s ± 1% -3.12% (p=0.004 n=6+5)
name old user-time/op new user-time/op delta
Build-8 11.9s ± 2% 10.8s ± 1% -8.71% (p=0.002 n=6+6)
While at it, only do CI builds on pushes and PRs to the master branch,
so that my PRs created from the same repo don't trigger duplicate
builds.
4 years ago
|
|
|
pull_request:
|
|
|
|
|
branches:
|
|
|
|
|
- master
|
|
|
|
|
|
|
|
|
|
# Note that a full "go test" is quite heavy,
|
|
|
|
|
# as it runs many builds under the hood.
|
|
|
|
|
# The default -timeout=10m can be hit by the hosted runners.
|
|
|
|
|
#
|
|
|
|
|
# Also note that we don't use actions/cache for Go on purpose.
|
|
|
|
|
# Caching GOMODCACHE wouldn't help much, as we have few deps.
|
|
|
|
|
# Caching GOCACHE would do more harm than good,
|
|
|
|
|
# as the tests redo most of their work if the garble version changes,
|
|
|
|
|
# and the majority of commits or PRs will do so.
|
|
|
|
|
|
|
|
|
|
name: Test
|
|
|
|
|
jobs:
|
|
|
|
|
test:
|
|
|
|
|
strategy:
|
|
|
|
|
matrix:
|
|
|
|
|
go-version: [1.17.x, 1.18.0-rc.1]
|
fix a data race with the global cachedBinary mechanism (#413)
Spotted by our friend "go test -race":
WARNING: DATA RACE
Write at 0x0000010522d8 by goroutine 69:
mvdan.cc/garble.readFile()
garble/main_test.go:124 +0x23a
mvdan.cc/garble.binsubstr()
garble/main_test.go:141 +0xc4
github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
[...]
Previous write at 0x0000010522d8 by goroutine 60:
mvdan.cc/garble.readFile()
garble/main_test.go:124 +0x23a
mvdan.cc/garble.binsubstr()
garble/main_test.go:141 +0xc4
github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
[...]
This wasn't a data race that we spotted via failures in practice,
as it only affected test code since July.
The race is due to the fact that each test script runs as a parallel
sub-test within the same Go program, sharing all globals.
As such, a single "cached binary" global is read and written with races.
Moreover, note that the caching always missed.
I briefly rewrote the code to avoid the race via a sync.Map keyed by
absolute filenames, and while that removed the data race,
the caching never actually hit.
To have a cache hit, we need an absolute path to already be in the cache
and for it to not have been modified since it was last cached. That is:
modify-bin-1 foo
binsubstr foo 'abc' # miss
binsubstr foo 'def' # hit; use the cached "/tmp/[...]/foo" entry
modify-bin-2 foo
binsubstr foo 'abc' # miss
However, the test scripts don't do contiguous binsubstr calls like
these. Instead, they join repeated binsubstr calls:
modify-bin-1 foo
binsubstr foo 'abc' 'def' # miss
modify-bin-2 foo
binsubstr foo 'abc' # miss
For that reason, remove the extra code entirely.
I didn't notice any change to the performance of "go test -short"
with a warm build cache, with:
go test -c
./garble.test -test.short #warm cache
benchcmd -n 5 TestShort ./garble.test -test.short
name old time/op new time/op delta
TestShort 4.62s ±12% 4.35s ±12% ~ (p=0.310 n=5+5)
name old user-time/op new user-time/op delta
TestShort 16.8s ± 3% 16.7s ± 3% ~ (p=0.690 n=5+5)
name old sys-time/op new sys-time/op delta
TestShort 7.28s ± 1% 7.26s ± 2% ~ (p=0.841 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
TestShort 305MB ± 0% 306MB ± 0% ~ (p=0.421 n=5+5)
Finally, start using "go test -race" on Linux on CI,
which should have made the PR back in July red before merging.
3 years ago
|
|
|
os: [ubuntu-latest, macos-latest, windows-latest]
|
|
|
|
|
runs-on: ${{ matrix.os }}
|
|
|
|
|
steps:
|
|
|
|
|
- uses: actions/setup-go@v3
|
|
|
|
|
with:
|
|
|
|
|
go-version: ${{ matrix.go-version }}
|
|
|
|
|
- uses: actions/checkout@v3
|
|
|
|
|
- name: Test
|
|
|
|
|
run: go test -timeout=15m ./...
|
fix a data race with the global cachedBinary mechanism (#413)
Spotted by our friend "go test -race":
WARNING: DATA RACE
Write at 0x0000010522d8 by goroutine 69:
mvdan.cc/garble.readFile()
garble/main_test.go:124 +0x23a
mvdan.cc/garble.binsubstr()
garble/main_test.go:141 +0xc4
github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
[...]
Previous write at 0x0000010522d8 by goroutine 60:
mvdan.cc/garble.readFile()
garble/main_test.go:124 +0x23a
mvdan.cc/garble.binsubstr()
garble/main_test.go:141 +0xc4
github.com/rogpeppe/go-internal/testscript.(*TestScript).run()
github.com/rogpeppe/go-internal@v1.8.1-0.20211023094830-115ce09fd6b4/testscript/testscript.go:496 +0x9e8
[...]
This wasn't a data race that we spotted via failures in practice,
as it only affected test code since July.
The race is due to the fact that each test script runs as a parallel
sub-test within the same Go program, sharing all globals.
As such, a single "cached binary" global is read and written with races.
Moreover, note that the caching always missed.
I briefly rewrote the code to avoid the race via a sync.Map keyed by
absolute filenames, and while that removed the data race,
the caching never actually hit.
To have a cache hit, we need an absolute path to already be in the cache
and for it to not have been modified since it was last cached. That is:
modify-bin-1 foo
binsubstr foo 'abc' # miss
binsubstr foo 'def' # hit; use the cached "/tmp/[...]/foo" entry
modify-bin-2 foo
binsubstr foo 'abc' # miss
However, the test scripts don't do contiguous binsubstr calls like
these. Instead, they join repeated binsubstr calls:
modify-bin-1 foo
binsubstr foo 'abc' 'def' # miss
modify-bin-2 foo
binsubstr foo 'abc' # miss
For that reason, remove the extra code entirely.
I didn't notice any change to the performance of "go test -short"
with a warm build cache, with:
go test -c
./garble.test -test.short #warm cache
benchcmd -n 5 TestShort ./garble.test -test.short
name old time/op new time/op delta
TestShort 4.62s ±12% 4.35s ±12% ~ (p=0.310 n=5+5)
name old user-time/op new user-time/op delta
TestShort 16.8s ± 3% 16.7s ± 3% ~ (p=0.690 n=5+5)
name old sys-time/op new sys-time/op delta
TestShort 7.28s ± 1% 7.26s ± 2% ~ (p=0.841 n=5+5)
name old peak-RSS-bytes new peak-RSS-bytes delta
TestShort 305MB ± 0% 306MB ± 0% ~ (p=0.421 n=5+5)
Finally, start using "go test -race" on Linux on CI,
which should have made the PR back in July red before merging.
3 years ago
|
|
|
- name: Test with -race
|
|
|
|
|
# macos and windows tend to be a bit slower,
|
|
|
|
|
# and it's rare that a race in garble would be OS-specific.
|
|
|
|
|
if: matrix.os == 'ubuntu-latest'
|
|
|
|
|
run: go test -race -timeout=20m ./...
|
|
|
|
|
|
|
|
|
|
# Static checks from this point forward. Only run on one Go version and on
|
|
|
|
|
# Linux, since it's the fastest platform, and the tools behave the same.
|
|
|
|
|
- if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.17.x'
|
|
|
|
|
run: diff <(echo -n) <(gofmt -d .)
|
|
|
|
|
- if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.17.x'
|
|
|
|
|
run: go vet ./...
|
|
|
|
|
- if: matrix.os == 'ubuntu-latest' && matrix.go-version == '1.17.x'
|
|
|
|
|
uses: dominikh/staticcheck-action@v1.1.0
|
|
|
|
|
with:
|
|
|
|
|
version: "2021.1.2"
|
|
|
|
|
install-go: false
|
|
|
|
|
|
|
|
|
|
# We don't care about GOARCH=386 particularly,
|
|
|
|
|
# but it helps ensure we support 32-bit hosts and targets well.
|
|
|
|
|
# TODO: use CGO_ENABLED=1 once we figure out how to install gcc-multilib,
|
|
|
|
|
# and once gotooltest forwards the value of CGO_ENABLED to the scripts.
|
|
|
|
|
test-386:
|
|
|
|
|
runs-on: ubuntu-latest
|
|
|
|
|
env:
|
|
|
|
|
GOARCH: 386
|
|
|
|
|
steps:
|
|
|
|
|
- uses: actions/setup-go@v3
|
|
|
|
|
with:
|
|
|
|
|
go-version: 1.17.x
|
|
|
|
|
- uses: actions/checkout@v3
|
|
|
|
|
- name: Test
|
|
|
|
|
run: go test -timeout=15m ./...
|
|
|
|
|
|
|
|
|
|
test-gotip:
|
|
|
|
|
runs-on: ubuntu-latest
|
|
|
|
|
steps:
|
|
|
|
|
- name: Install Go
|
CI: pin a commit when testing against Go tip
Since it changes rapidly, especially during merge cycles, and we don't
want CI to surprisingly blow up in our faces from one day to another.
Pin this to a commit from yesterday which works, since some changes
merged today moved where the Go build version is recorded and broke
garble.
While at it, replace "git clone" with a wget of a source archive. This
is much, much faster, mainly because a tarball is significantly smaller.
We now download about 20MiB instead of over 350MiB.
One downside is that, without git, make.bash can't construct a devel
version on its own. For that reason, add a pretty basic manual version
via the VERSION file.
This means that we must not reject custom devel version strings. This is
a good thing anyway, because custom devel strings are already common
when building Go in custom ways. Those people tend to be advanced users,
such as CI, so fall back to assuming they know what they are doing and
don't error.
Plus, starting last week, devel versions in Go master now contain the
major Go version like in build tags, such as "go1.17-commit...", so we
will soon start relying on that instead of parsing dates:
$ go version
go version devel go1.17-a7e16abb22 Thu Apr 8 07:33:58 2021 +0000 linux/amd64
3 years ago
|
|
|
env:
|
|
|
|
|
GO_COMMIT: 86b5f6a7be707b9d84108df6f459c7e84bf9dbac # 2022-03-02
|
|
|
|
|
run: |
|
CI: pin a commit when testing against Go tip
Since it changes rapidly, especially during merge cycles, and we don't
want CI to surprisingly blow up in our faces from one day to another.
Pin this to a commit from yesterday which works, since some changes
merged today moved where the Go build version is recorded and broke
garble.
While at it, replace "git clone" with a wget of a source archive. This
is much, much faster, mainly because a tarball is significantly smaller.
We now download about 20MiB instead of over 350MiB.
One downside is that, without git, make.bash can't construct a devel
version on its own. For that reason, add a pretty basic manual version
via the VERSION file.
This means that we must not reject custom devel version strings. This is
a good thing anyway, because custom devel strings are already common
when building Go in custom ways. Those people tend to be advanced users,
such as CI, so fall back to assuming they know what they are doing and
don't error.
Plus, starting last week, devel versions in Go master now contain the
major Go version like in build tags, such as "go1.17-commit...", so we
will soon start relying on that instead of parsing dates:
$ go version
go version devel go1.17-a7e16abb22 Thu Apr 8 07:33:58 2021 +0000 linux/amd64
3 years ago
|
|
|
cd $HOME
|
|
|
|
|
mkdir $HOME/gotip
|
|
|
|
|
cd $HOME/gotip
|
|
|
|
|
|
|
|
|
|
wget -O gotip.tar.gz https://go.googlesource.com/go/+archive/${GO_COMMIT}.tar.gz
|
|
|
|
|
tar -xf gotip.tar.gz
|
|
|
|
|
echo "devel go1.19-${GO_COMMIT}" >VERSION
|
CI: pin a commit when testing against Go tip
Since it changes rapidly, especially during merge cycles, and we don't
want CI to surprisingly blow up in our faces from one day to another.
Pin this to a commit from yesterday which works, since some changes
merged today moved where the Go build version is recorded and broke
garble.
While at it, replace "git clone" with a wget of a source archive. This
is much, much faster, mainly because a tarball is significantly smaller.
We now download about 20MiB instead of over 350MiB.
One downside is that, without git, make.bash can't construct a devel
version on its own. For that reason, add a pretty basic manual version
via the VERSION file.
This means that we must not reject custom devel version strings. This is
a good thing anyway, because custom devel strings are already common
when building Go in custom ways. Those people tend to be advanced users,
such as CI, so fall back to assuming they know what they are doing and
don't error.
Plus, starting last week, devel versions in Go master now contain the
major Go version like in build tags, such as "go1.17-commit...", so we
will soon start relying on that instead of parsing dates:
$ go version
go version devel go1.17-a7e16abb22 Thu Apr 8 07:33:58 2021 +0000 linux/amd64
3 years ago
|
|
|
|
|
|
|
|
cd src
|
|
|
|
|
# GOGC=off helps Go build about 20% faster, if we can spare memory.
|
|
|
|
|
GOGC=off ./make.bash
|
|
|
|
|
echo "GOROOT=$HOME/gotip" >>$GITHUB_ENV
|
|
|
|
|
echo "$HOME/gotip/bin" >>$GITHUB_PATH
|
|
|
|
|
- uses: actions/checkout@v3
|
|
|
|
|
- name: Test
|
|
|
|
|
run: go test -timeout=15m ./...
|
|
|
|
|
|
|
|
|
|
script-checks:
|
|
|
|
|
runs-on: ubuntu-latest
|
|
|
|
|
steps:
|
|
|
|
|
- uses: actions/checkout@v3
|
|
|
|
|
- run: ./scripts/crlf-test.sh
|
