From 40acf238f3340e7caad724a4e9ded6565db061fa Mon Sep 17 00:00:00 2001 From: Jarek Kowalski Date: Thu, 30 Jul 2020 17:31:28 -0700 Subject: [PATCH] Fixed arm and arm64 build. (#506) * fixed a number of cases where misaligned data was causing panics on armv7 (but not armv8) * travis: enable arm64 * test: reduce compressed data sizes when running on arm * arm: wait longer for snapshots --- .travis.yml | 3 ++ Makefile | 34 ++++++++++++++--- internal/faketime/faketime.go | 2 +- internal/stats/countsum.go | 3 ++ internal/stats/countsum_mutex.go | 37 +++++++++++++++++++ repo/content/content_cache_base.go | 12 +++--- repo/content/content_manager.go | 1 + repo/content/content_manager_lock_free.go | 3 +- repo/object/object_manager_test.go | 12 +++++- snapshot/snapshotfs/upload.go | 5 ++- tests/end_to_end_test/server_start_test.go | 4 +- tests/robustness/main_test.go | 2 +- tests/robustness/robustness_test.go | 2 +- tests/robustness/test_engine/engine.go | 2 +- tests/robustness/test_engine/engine_test.go | 2 +- tests/tools/fswalker/fswalker.go | 2 +- tests/tools/fswalker/fswalker_test.go | 2 +- tests/tools/fswalker/reporter/reporter.go | 2 +- .../tools/fswalker/reporter/reporter_test.go | 2 +- tests/tools/fswalker/walker/walker.go | 2 +- tests/tools/fswalker/walker/walker_test.go | 2 +- tools/tools.mk | 35 ++++++++++++++++-- 22 files changed, 137 insertions(+), 34 deletions(-) create mode 100644 internal/stats/countsum_mutex.go diff --git a/.travis.yml b/.travis.yml index 1c5519722..9ac40c53f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,9 @@ go: os: - linux - osx +arch: +- amd64 +- arm64 addons: apt: packages: diff --git a/Makefile b/Makefile index c24843fef..2df9b50c9 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,5 @@ COVERAGE_PACKAGES=github.com/kopia/kopia/repo/...,github.com/kopia/kopia/fs/...,github.com/kopia/kopia/snapshot/... GO_TEST=go test -PARALLEL=8 TEST_FLAGS?= KOPIA_INTEGRATION_EXE=$(CURDIR)/dist/integration/kopia.exe FIO_DOCKER_TAG=ljishen/fio @@ -15,6 +14,17 @@ endif include tools/tools.mk +ifeq ($(kopia_arch_name),amd64) +PARALLEL=8 +LINTER_DEADLINE=180s +UNIT_TESTS_TIMEOUT=180s +else +# tweaks for less powerful platforms +PARALLEL=2 +LINTER_DEADLINE=300s +UNIT_TESTS_TIMEOUT=300s +endif + -include ./Makefile.local.mk install: html-ui-bindata @@ -39,10 +49,10 @@ play: go run cmd/playground/main.go lint: $(linter) - $(linter) --deadline 180s run $(linter_flags) + $(linter) --deadline $(LINTER_DEADLINE) run $(linter_flags) lint-and-log: $(linter) - $(linter) --deadline 180s run $(linter_flags) | tee .linterr.txt + $(linter) --deadline $(LINTER_DEADLINE) run $(linter_flags) | tee .linterr.txt vet-time-inject: @@ -58,7 +68,10 @@ vet: vet-time-inject travis-setup: travis-install-gpg-key travis-install-test-credentials all-tools go mod download make -C htmlui node_modules +ifeq ($(kopia_arch_name),amd64) make -C app node_modules +endif + ifneq ($(TRAVIS_OS_NAME),) -git checkout go.mod go.sum endif @@ -81,6 +94,13 @@ html-ui-bindata-fallback: $(go_bindata) kopia-ui: $(MAKE) -C app build-electron +ifeq ($(kopia_arch_name),arm64) +travis-release: + $(MAKE) test + $(MAKE) integration-tests + $(MAKE) lint +else + travis-release: $(retry) $(MAKE) goreleaser $(retry) $(MAKE) kopia-ui @@ -95,6 +115,8 @@ ifeq ($(TRAVIS_OS_NAME),linux) $(MAKE) upload-coverage endif +endif + # goreleaser - builds binaries for all platforms GORELEASER_OPTIONS=--rm-dist --parallelism=6 @@ -165,17 +187,17 @@ test-with-coverage-pkgonly: $(GO_TEST) -count=1 -coverprofile=tmp.cov -timeout 90s github.com/kopia/kopia/... test: - $(GO_TEST) -count=1 -timeout 180s ./... + $(GO_TEST) -count=1 -timeout $(UNIT_TESTS_TIMEOUT) ./... vtest: - $(GO_TEST) -count=1 -short -v -timeout 180s ./... + $(GO_TEST) -count=1 -short -v -timeout $(UNIT_TESTS_TIMEOUT) ./... dist-binary: go build -o $(KOPIA_INTEGRATION_EXE) github.com/kopia/kopia integration-tests: export KOPIA_EXE ?= $(KOPIA_INTEGRATION_EXE) integration-tests: dist-binary - $(GO_TEST) $(TEST_FLAGS) -count=1 -parallel $(PARALLEL) -timeout 600s github.com/kopia/kopia/tests/end_to_end_test + $(GO_TEST) $(TEST_FLAGS) -count=1 -parallel $(PARALLEL) -timeout 3600s github.com/kopia/kopia/tests/end_to_end_test robustness-tool-tests: FIO_DOCKER_IMAGE=$(FIO_DOCKER_TAG) \ diff --git a/internal/faketime/faketime.go b/internal/faketime/faketime.go index e97d7d254..c67b7b804 100644 --- a/internal/faketime/faketime.go +++ b/internal/faketime/faketime.go @@ -35,8 +35,8 @@ func AutoAdvance(t time.Time, dt time.Duration) func() time.Time { // TimeAdvance allows controlling the passage of time. Intended to be used in // tests. type TimeAdvance struct { - base time.Time delta int64 + base time.Time } // NewTimeAdvance creates a TimeAdvance with the given start time diff --git a/internal/stats/countsum.go b/internal/stats/countsum.go index 430d4258f..db803545a 100644 --- a/internal/stats/countsum.go +++ b/internal/stats/countsum.go @@ -1,4 +1,7 @@ +// +build amd64 + // Package stats provides helpers for simple stats +// package stats import "sync/atomic" diff --git a/internal/stats/countsum_mutex.go b/internal/stats/countsum_mutex.go new file mode 100644 index 000000000..57dce7406 --- /dev/null +++ b/internal/stats/countsum_mutex.go @@ -0,0 +1,37 @@ +// +build !amd64 + +// Package stats provides helpers for simple stats. This implementation uses mutex to work around +// ARM alignment issues with CountSum due to unpredictable memory alignment. +package stats + +import ( + "sync" +) + +// CountSum holds sum and count values +type CountSum struct { + mu sync.Mutex + sum int64 + count uint32 +} + +// Add adds size to s and returns approximate values for the current count +// and total bytes +func (s *CountSum) Add(size int64) (count uint32, sum int64) { + s.mu.Lock() + defer s.mu.Unlock() + + s.count++ + s.sum += size + + return s.count, s.sum +} + +// Approximate returns an approximation of the current count and sum values. +// It is approximate because retrieving both values is not an atomic operation. +func (s *CountSum) Approximate() (count uint32, sum int64) { + s.mu.Lock() + defer s.mu.Unlock() + + return s.count, s.sum +} diff --git a/repo/content/content_cache_base.go b/repo/content/content_cache_base.go index 731476702..6b76a3fda 100644 --- a/repo/content/content_cache_base.go +++ b/repo/content/content_cache_base.go @@ -18,9 +18,11 @@ mutexAgeCutoff = 5 * time.Minute ) -type mutextLRU struct { - mu *sync.Mutex +type mutexLRU struct { + // values aligned to 8-bytes due to atomic access lastUsedNanoseconds int64 + + mu *sync.Mutex } // cacheBase provides common implementation for per-content and per-blob caches @@ -58,13 +60,13 @@ func (c *cacheBase) perItemMutex(key interface{}) *sync.Mutex { v, ok := c.loadingMap.Load(key) if !ok { - v, _ = c.loadingMap.LoadOrStore(key, &mutextLRU{ + v, _ = c.loadingMap.LoadOrStore(key, &mutexLRU{ mu: &sync.Mutex{}, lastUsedNanoseconds: now, }) } - m := v.(*mutextLRU) + m := v.(*mutexLRU) atomic.StoreInt64(&m.lastUsedNanoseconds, now) return m.mu @@ -151,7 +153,7 @@ func (c *cacheBase) sweepMutexes() { // since the mutexes are only for performance (to avoid loading duplicates) // and not for correctness, it's always safe to remove them. c.loadingMap.Range(func(key, value interface{}) bool { - if m := value.(*mutextLRU); m.lastUsedNanoseconds < cutoffTime { + if m := value.(*mutexLRU); m.lastUsedNanoseconds < cutoffTime { c.loadingMap.Delete(key) } diff --git a/repo/content/content_manager.go b/repo/content/content_manager.go index 734630bcb..b1513557f 100644 --- a/repo/content/content_manager.go +++ b/repo/content/content_manager.go @@ -700,6 +700,7 @@ func newManagerWithOptions(ctx context.Context, st blob.Storage, f *FormattingOp mu := &sync.RWMutex{} m := &Manager{ lockFreeManager: lockFreeManager{ + Stats: new(Stats), Format: *f, timeNow: timeNow, maxPackSize: f.MaxPackSize, diff --git a/repo/content/content_manager_lock_free.go b/repo/content/content_manager_lock_free.go index 37a62fe92..18ed521cc 100644 --- a/repo/content/content_manager_lock_free.go +++ b/repo/content/content_manager_lock_free.go @@ -23,8 +23,7 @@ // lockFreeManager contains parts of Manager state that can be accessed without locking type lockFreeManager struct { - // this one is not lock-free - Stats Stats + Stats *Stats st blob.Storage Format FormattingOptions diff --git a/repo/object/object_manager_test.go b/repo/object/object_manager_test.go index 6d6e71bb5..e31d229df 100644 --- a/repo/object/object_manager_test.go +++ b/repo/object/object_manager_test.go @@ -11,6 +11,7 @@ "io" "io/ioutil" "math/rand" + "runtime" "runtime/debug" "sync" "testing" @@ -343,6 +344,13 @@ func TestEndToEndReadAndSeek(t *testing.T) { } func TestEndToEndReadAndSeekWithCompression(t *testing.T) { + sizes := []int{1, 199, 200, 201, 9999, 512434, 5012434, 15000000} + asyncWritesList := []int{0, 4, 8} + + if runtime.GOARCH != "amd64" { + sizes = []int{1, 199, 200, 201, 9999, 512434} + } + for _, compressible := range []bool{false, true} { compressible := compressible @@ -353,14 +361,14 @@ func TestEndToEndReadAndSeekWithCompression(t *testing.T) { ctx := testlogging.Context(t) - for _, asyncWrites := range []int{0, 4, 8} { + for _, asyncWrites := range asyncWritesList { asyncWrites := asyncWrites totalBytesWritten := 0 data, om := setupTest(t) t.Run(fmt.Sprintf("async-%v", asyncWrites), func(t *testing.T) { t.Parallel() - for _, size := range []int{1, 199, 200, 201, 9999, 512434, 5012434, 15000000} { + for _, size := range sizes { randomData := makeMaybeCompressibleData(size, compressible) diff --git a/snapshot/snapshotfs/upload.go b/snapshot/snapshotfs/upload.go index ee648aa05..2bc1f021c 100644 --- a/snapshot/snapshotfs/upload.go +++ b/snapshot/snapshotfs/upload.go @@ -45,6 +45,9 @@ // Uploader supports efficient uploading files and directories to repository. type Uploader struct { + // values aligned to 8-bytes due to atomic access + totalWrittenBytes int64 + Progress UploadProgress // automatically cancel the Upload after certain number of bytes @@ -71,8 +74,6 @@ type Uploader struct { nextCheckpointTime time.Time uploadBufPool sync.Pool - - totalWrittenBytes int64 } // IsCanceled returns true if the upload is canceled. diff --git a/tests/end_to_end_test/server_start_test.go b/tests/end_to_end_test/server_start_test.go index 061540871..2e79d9a96 100644 --- a/tests/end_to_end_test/server_start_test.go +++ b/tests/end_to_end_test/server_start_test.go @@ -268,7 +268,7 @@ func verifyServerConnected(t *testing.T, cli *apiclient.KopiaAPIClient, want boo func waitForSnapshotCount(ctx context.Context, t *testing.T, cli *apiclient.KopiaAPIClient, match *snapshot.SourceInfo, want int) { t.Helper() - err := retry.PeriodicallyNoValue(ctx, 1*time.Second, 60, "wait for snapshots", func() error { + err := retry.PeriodicallyNoValue(ctx, 1*time.Second, 180, "wait for snapshots", func() error { snapshots, err := serverapi.ListSnapshots(testlogging.Context(t), cli, match) if err != nil { return errors.Wrap(err, "error listing sources") @@ -359,7 +359,7 @@ func verifyUIServedWithCorrectTitle(t *testing.T, cli *apiclient.KopiaAPIClient, } func waitUntilServerStarted(ctx context.Context, t *testing.T, cli *apiclient.KopiaAPIClient) { - if err := retry.PeriodicallyNoValue(ctx, 1*time.Second, 60, "wait for server start", func() error { + if err := retry.PeriodicallyNoValue(ctx, 1*time.Second, 180, "wait for server start", func() error { _, err := serverapi.Status(testlogging.Context(t), cli) return err }, retry.Always); err != nil { diff --git a/tests/robustness/main_test.go b/tests/robustness/main_test.go index 03ae2c6d1..f3645ac89 100644 --- a/tests/robustness/main_test.go +++ b/tests/robustness/main_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 package robustness diff --git a/tests/robustness/robustness_test.go b/tests/robustness/robustness_test.go index 68a3b4757..8c5a8815d 100644 --- a/tests/robustness/robustness_test.go +++ b/tests/robustness/robustness_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 package robustness diff --git a/tests/robustness/test_engine/engine.go b/tests/robustness/test_engine/engine.go index 8e0ce73b5..366479b6a 100644 --- a/tests/robustness/test_engine/engine.go +++ b/tests/robustness/test_engine/engine.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 // Package engine provides the framework for a snapshot repository testing engine package engine diff --git a/tests/robustness/test_engine/engine_test.go b/tests/robustness/test_engine/engine_test.go index c34fc5b43..57a09aeb3 100644 --- a/tests/robustness/test_engine/engine_test.go +++ b/tests/robustness/test_engine/engine_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 // Package engine provides the framework for a snapshot repository testing engine package engine diff --git a/tests/tools/fswalker/fswalker.go b/tests/tools/fswalker/fswalker.go index bb9b0d51f..4b3b9e15b 100644 --- a/tests/tools/fswalker/fswalker.go +++ b/tests/tools/fswalker/fswalker.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 // Package fswalker provides the checker.Comparer interface using FSWalker // walker and reporter. diff --git a/tests/tools/fswalker/fswalker_test.go b/tests/tools/fswalker/fswalker_test.go index 67550e6fc..54bd57682 100644 --- a/tests/tools/fswalker/fswalker_test.go +++ b/tests/tools/fswalker/fswalker_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 package fswalker diff --git a/tests/tools/fswalker/reporter/reporter.go b/tests/tools/fswalker/reporter/reporter.go index 6d184a743..73cbc4d1f 100644 --- a/tests/tools/fswalker/reporter/reporter.go +++ b/tests/tools/fswalker/reporter/reporter.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 // Package reporter wraps calls to the the fswalker Reporter package reporter diff --git a/tests/tools/fswalker/reporter/reporter_test.go b/tests/tools/fswalker/reporter/reporter_test.go index cf32933d8..a9cedf06a 100644 --- a/tests/tools/fswalker/reporter/reporter_test.go +++ b/tests/tools/fswalker/reporter/reporter_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 package reporter diff --git a/tests/tools/fswalker/walker/walker.go b/tests/tools/fswalker/walker/walker.go index 150fbecb1..4db5b50bb 100644 --- a/tests/tools/fswalker/walker/walker.go +++ b/tests/tools/fswalker/walker/walker.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 // Package walker wraps calls to the the fswalker Walker package walker diff --git a/tests/tools/fswalker/walker/walker_test.go b/tests/tools/fswalker/walker/walker_test.go index fa3957fc8..6c38532ed 100644 --- a/tests/tools/fswalker/walker/walker_test.go +++ b/tests/tools/fswalker/walker/walker_test.go @@ -1,4 +1,4 @@ -// +build darwin linux +// +build darwin,amd64 linux,amd64 package walker diff --git a/tools/tools.mk b/tools/tools.mk index 00e3677c8..71b1fb0d9 100644 --- a/tools/tools.mk +++ b/tools/tools.mk @@ -10,6 +10,26 @@ ifeq ($(TRAVIS_OS_NAME),windows) UNIX_SHELL_ON_WINDOWS=true endif +kopia_arch_name=amd64 +node_arch_name=x64 +goreleaser_arch_name=x86_64 +linter_arch_name=amd64 + +raw_arch:=$(shell uname -m) +ifeq ($(raw_arch),aarch64) + kopia_arch_name=arm64 + node_arch_name=arm64 + goreleaser_arch_name=arm64 + linter_arch_name=arm64 +endif + +ifeq ($(raw_arch),armv7l) + kopia_arch_name=arm + node_arch_name=armv7l + goreleaser_arch_name=armv6 + linter_arch_name=armv7 +endif + ifneq ($(APPVEYOR),) UNIX_SHELL_ON_WINDOWS=false @@ -121,7 +141,7 @@ ifeq ($(uname),Windows) else ifeq ($(uname),Linux) - curl -LsS https://nodejs.org/dist/v$(NODE_VERSION)/node-v$(NODE_VERSION)-linux-x64.tar.gz | tar zx -C $(node_base_dir) + curl -LsS https://nodejs.org/dist/v$(NODE_VERSION)/node-v$(NODE_VERSION)-linux-$(node_arch_name).tar.gz | tar zx -C $(node_base_dir) else curl -LsS https://nodejs.org/dist/v$(NODE_VERSION)/node-v$(NODE_VERSION)-darwin-x64.tar.gz | tar zx -C $(node_base_dir) endif @@ -154,7 +174,7 @@ ifeq ($(uname),Windows) else mkdir -p $(linter_dir) ifeq ($(uname),Linux) - curl -LsS https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/golangci-lint-$(GOLANGCI_LINT_VERSION)-linux-amd64.tar.gz | tar zxv --strip=1 -C $(linter_dir) + curl -LsS https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/golangci-lint-$(GOLANGCI_LINT_VERSION)-linux-$(linter_arch_name).tar.gz | tar zxv --strip=1 -C $(linter_dir) else curl -LsS https://github.com/golangci/golangci-lint/releases/download/v$(GOLANGCI_LINT_VERSION)/golangci-lint-$(GOLANGCI_LINT_VERSION)-darwin-amd64.tar.gz | tar zxv --strip=1 -C $(linter_dir) endif @@ -193,7 +213,7 @@ ifeq ($(uname),Windows) curl -LsS -o $(goreleaser_dir).zip https://github.com/goreleaser/goreleaser/releases/download/$(GORELEASER_VERSION)/goreleaser_Windows_x86_64.zip unzip -q $(goreleaser_dir).zip -d $(goreleaser_dir) else - curl -LsS https://github.com/goreleaser/goreleaser/releases/download/$(GORELEASER_VERSION)/goreleaser_$$(uname -s)_$$(uname -m).tar.gz | tar zx -C $(TOOLS_DIR)/goreleaser-$(GORELEASER_VERSION) + curl -LsS https://github.com/goreleaser/goreleaser/releases/download/$(GORELEASER_VERSION)/goreleaser_$$(uname -s)_$(goreleaser_arch_name).tar.gz | tar zx -C $(TOOLS_DIR)/goreleaser-$(GORELEASER_VERSION) endif ifeq ($(TRAVIS_PULL_REQUEST),false) @@ -235,5 +255,12 @@ else endif endif -all-tools: $(npm) $(goreleaser) $(linter) $(hugo) $(go_bindata) windows-signing-tools +# disable some tools on non-default architectures +ifeq ($(kopia_arch_name),amd64) +maybehugo=$(hugo) +else +maybehugo= +endif + +all-tools: $(npm) $(goreleaser) $(linter) $(maybehugo) $(go_bindata) windows-signing-tools