Skip to content

Commit

Permalink
chore: optimize maps.Values and maps.Keys
Browse files Browse the repository at this point in the history
This PR optimizes the performance of maps.Values and maps.Keys (around 125 usages in sidero code)
by using the internal runtime functions runtime.keys and runtime.values. There is strong desire in Go Team to disable
`go:linkname` usage in user code [^1], but this is a special case where we use `Push` `linkname` pattern which is said
to be supported in the future. It is tested with both Go 1.22 and latest Go with CL 585556 [^2] applied.

To further future-proof this code, we have added a build tag `go1.22 && !go1.24` to ensure this code is only compiled
with Go 1.22 and 1.23 and falls back to the old implementation for Go 1.24 and above. This is because we don't know yet
if `runtime.keys` and `runtime.values` are going to be present in Go 1.24. We will update this code when Go 1.24 freeze
happens.

Benchstat results below (overall 26% CPU usage reduction):
```bash
~ benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/siderolabs/gen/maps
                │   old.txt    │               new.txt               │
                │    sec/op    │   sec/op     vs base                │
Keys/small-10     111.05n ± 0%   80.74n ± 3%  -27.29% (p=0.000 n=10)
Keys/mid-10        837.8n ± 1%   607.8n ± 2%  -27.46% (p=0.000 n=10)
Keys/large-10      7.717µ ± 4%   5.711µ ± 0%  -26.00% (p=0.000 n=10)
Values/small-10   110.40n ± 0%   83.69n ± 0%  -24.19% (p=0.000 n=10)
Values/mid-10      835.9n ± 1%   583.5n ± 1%  -30.19% (p=0.000 n=10)
Values/large-10    7.720µ ± 2%   5.696µ ± 1%  -26.22% (p=0.000 n=10)
geomean            894.3n        653.6n       -26.92%

                │   old.txt    │                new.txt                │
                │     B/op     │     B/op      vs base                 │
Keys/small-10       80.00 ± 0%     80.00 ± 0%       ~ (p=1.000 n=10) ¹
Keys/mid-10         896.0 ± 0%     896.0 ± 0%       ~ (p=1.000 n=10) ¹
Keys/large-10     8.000Ki ± 0%   8.000Ki ± 0%       ~ (p=1.000 n=10) ¹
Values/small-10     80.00 ± 0%     80.00 ± 0%       ~ (p=1.000 n=10) ¹
Values/mid-10       896.0 ± 0%     896.0 ± 0%       ~ (p=1.000 n=10) ¹
Values/large-10   8.000Ki ± 0%   8.000Ki ± 0%       ~ (p=1.000 n=10) ¹
geomean             837.4          837.4       +0.00%
¹ all samples are equal

                │  old.txt     │                new.txt                │
                │ allocs/op    │  allocs/op    vs base                 │
Keys/small-10     1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
Keys/mid-10       1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
Keys/large-10     1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
Values/small-10   1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
Values/mid-10     1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
Values/large-10   1.000 ± 0%     1.000 ± 0%         ~ (p=1.000 n=10) ¹
geomean           1.000          1.000         +0.00%
¹ all samples are equal
```

[^1]: golang/go#67401
[^2]: https://go-review.googlesource.com/c/go/+/585556

Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
  • Loading branch information
DmitriyMV committed May 19, 2024
1 parent 238baf9 commit 8485864
Show file tree
Hide file tree
Showing 13 changed files with 238 additions and 140 deletions.
2 changes: 1 addition & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2024-03-11T19:57:58Z by kres latest.
# Generated on 2024-05-19T20:59:37Z by kres dccd292.

*
!channel
Expand Down
14 changes: 9 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2024-03-11T19:57:58Z by kres latest.
# Generated on 2024-05-19T18:27:53Z by kres dccd292.

name: default
concurrency:
Expand Down Expand Up @@ -31,7 +31,7 @@ jobs:
if: (!startsWith(github.head_ref, 'renovate/') && !startsWith(github.head_ref, 'dependabot/'))
services:
buildkitd:
image: moby/buildkit:v0.12.5
image: moby/buildkit:v0.13.2
options: --privileged
ports:
- 1234:1234
Expand All @@ -45,11 +45,12 @@ jobs:
run: |
git fetch --prune --unshallow
- name: Set up Docker Buildx
id: setup-buildx
uses: docker/setup-buildx-action@v3
with:
driver: remote
endpoint: tcp://127.0.0.1:1234
timeout-minutes: 1
timeout-minutes: 10
- name: base
run: |
make base
Expand All @@ -60,8 +61,11 @@ jobs:
run: |
make unit-tests-race
- name: coverage
run: |
make coverage
uses: codecov/codecov-action@v4
with:
files: _out/coverage-unit-tests.txt
token: ${{ secrets.CODECOV_TOKEN }}
timeout-minutes: 3
- name: lint
run: |
make lint
Expand Down
83 changes: 32 additions & 51 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2024-03-11T19:57:58Z by kres latest.
# Generated on 2024-05-19T20:58:19Z by kres dccd292.

# options for analysis running
run:
timeout: 10m
issues-exit-code: 1
tests: true
build-tags: []
skip-dirs: []
skip-dirs-use-default: true
skip-files: []
build-tags: [ ]
modules-download-mode: readonly

# output configuration options
output:
format: colored-line-number
formats:
- format: colored-line-number
path: stdout
print-issued-lines: true
print-linter-name: true
uniq-by-line: true
Expand All @@ -32,54 +31,35 @@ linters-settings:
check-blank: true
exhaustive:
default-signifies-exhaustive: false
funlen:
lines: 60
statements: 40
gci:
local-prefixes: github.com/siderolabs/gen/
sections:
- standard # Standard section: captures all standard packages.
- default # Default section: contains all imports that could not be matched to another section type.
- localmodule # Imports from the same module.
gocognit:
min-complexity: 30
ireturn:
allow:
- anon
- error
- empty
- stdlib
- github.com\/talos-systems\/kres\/internal\/dag.Node
nestif:
min-complexity: 5
goconst:
min-len: 3
min-occurrences: 3
gocritic:
disabled-checks: []
disabled-checks: [ ]
gocyclo:
min-complexity: 20
godot:
check-all: false
godox:
keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting
- NOTE
- OPTIMIZE # marks code that should be optimized before merging
- HACK # marks hack-arounds that should be removed before merging
scope: declarations
gofmt:
simplify: true
goimports:
local-prefixes: github.com/siderolabs/gen/
golint:
min-confidence: 0.8
gomnd:
settings: {}
gomodguard: {}
gomodguard: { }
govet:
check-shadowing: true
enable-all: true
lll:
line-length: 200
tab-width: 4
misspell:
locale: US
ignore-words: []
ignore-words: [ ]
nakedret:
max-func-lines: 30
prealloc:
Expand All @@ -88,16 +68,15 @@ linters-settings:
for-loops: false # Report preallocation suggestions on for loops, false by default
nolintlint:
allow-unused: false
allow-leading-space: false
allow-no-explanation: []
allow-no-explanation: [ ]
require-explanation: false
require-specific: true
rowserrcheck: {}
testpackage: {}
rowserrcheck: { }
testpackage: { }
unparam:
check-exported: false
unused:
check-exported: false
local-variables-are-used: false
whitespace:
multi-if: false # Enforces newlines (or comments) after every multi-line if statement
multi-func: false # Enforces newlines (or comments) after every multi-line function signature
Expand All @@ -113,8 +92,8 @@ linters-settings:
gofumpt:
extra-rules: false
cyclop:
# the maximal code complexity to report
max-complexity: 20
# the maximal code complexity to report
max-complexity: 20
# depguard:
# Main:
# deny:
Expand All @@ -125,48 +104,50 @@ linters:
disable-all: false
fast: false
disable:
- exhaustruct
- exhaustivestruct
- exhaustruct
- err113
- forbidigo
- funlen
- gas
- gochecknoglobals
- gochecknoinits
- godox
- goerr113
- gomnd
- gomoddirectives
- gosec
- inamedparam
- ireturn
- mnd
- nestif
- nonamedreturns
- nosnakecase
- paralleltest
- tagalign
- tagliatelle
- thelper
- typecheck
- varnamelen
- wrapcheck
- depguard # Disabled because starting with golangci-lint 1.53.0 it doesn't allow denylist alone anymore
- tagalign
- inamedparam
- testifylint # complains about our assert recorder and has a number of false positives for assert.Greater(t, thing, 1)
- protogetter # complains about us using Value field on typed spec, instead of GetValue which has a different signature
- perfsprint # complains about us using fmt.Sprintf in non-performance critical code, updating just kres took too long
# abandoned linters for which golangci shows the warning that the repo is archived by the owner
- deadcode
- golint
- ifshort
- interfacer
- maligned
- golint
- scopelint
- varcheck
- deadcode
- structcheck
- ifshort
- varcheck
# disabled as it seems to be broken - goes into imported libraries and reports issues there
- musttag
- goimports # same as gci

issues:
exclude: []
exclude-rules: []
exclude: [ ]
exclude-rules: [ ]
exclude-use-default: false
exclude-case-sensitive: false
max-issues-per-linter: 10
Expand Down
14 changes: 4 additions & 10 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
# syntax = docker/dockerfile-upstream:1.7.0-labs
# syntax = docker/dockerfile-upstream:1.7.1-labs

# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2024-03-11T19:57:58Z by kres latest.
# Generated on 2024-05-19T20:59:37Z by kres dccd292.

ARG TOOLCHAIN

# cleaned up specs and compiled versions
FROM scratch AS generate

# runs markdownlint
FROM docker.io/node:21.6.2-alpine3.19 AS lint-markdown
FROM docker.io/node:21.7.3-alpine3.19 AS lint-markdown
WORKDIR /src
RUN npm i -g markdownlint-cli@0.39.0
RUN npm i sentences-per-line@0.2.1
Expand Down Expand Up @@ -40,9 +40,6 @@ RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/g
&& mv /go/bin/golangci-lint /bin/golangci-lint
RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg go install golang.org/x/vuln/cmd/govulncheck@latest \
&& mv /go/bin/govulncheck /bin/govulncheck
ARG GOIMPORTS_VERSION
RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/go/pkg go install golang.org/x/tools/cmd/goimports@${GOIMPORTS_VERSION} \
&& mv /go/bin/goimports /bin/goimports
ARG GOFUMPT_VERSION
RUN go install mvdan.cc/gofumpt@${GOFUMPT_VERSION} \
&& mv /go/bin/gofumpt /bin/gofumpt
Expand Down Expand Up @@ -71,15 +68,12 @@ RUN --mount=type=cache,target=/go/pkg go list -mod=readonly all >/dev/null
FROM base AS lint-gofumpt
RUN FILES="$(gofumpt -l .)" && test -z "${FILES}" || (echo -e "Source code is not formatted with 'gofumpt -w .':\n${FILES}"; exit 1)

# runs goimports
FROM base AS lint-goimports
RUN FILES="$(goimports -l -local github.com/siderolabs/gen/ .)" && test -z "${FILES}" || (echo -e "Source code is not formatted with 'goimports -w -local github.com/siderolabs/gen/ .':\n${FILES}"; exit 1)

# runs golangci-lint
FROM base AS lint-golangci-lint
WORKDIR /src
COPY .golangci.yml .
ENV GOGC 50
RUN golangci-lint config verify --config .golangci.yml
RUN --mount=type=cache,target=/root/.cache/go-build --mount=type=cache,target=/root/.cache/golangci-lint --mount=type=cache,target=/go/pkg golangci-lint run --config .golangci.yml

# runs govulncheck
Expand Down
30 changes: 14 additions & 16 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2024-03-11T19:51:28Z by kres latest.
# Generated on 2024-05-19T18:27:53Z by kres dccd292.

# common variables

Expand All @@ -9,6 +9,9 @@ TAG := $(shell git describe --tag --always --dirty --match v[0-9]\*)
ABBREV_TAG := $(shell git describe --tags >/dev/null 2>/dev/null && git describe --tag --always --match v[0-9]\* --abbrev=0 || echo 'undefined')
BRANCH := $(shell git rev-parse --abbrev-ref HEAD)
ARTIFACTS := _out
IMAGE_TAG ?= $(TAG)
OPERATING_SYSTEM := $(shell uname -s | tr '[:upper:]' '[:lower:]')
GOARCH := $(shell uname -m | sed 's/x86_64/amd64/' | sed 's/aarch64/arm64/')
WITH_DEBUG ?= false
WITH_RACE ?= false
REGISTRY ?= ghcr.io
Expand All @@ -18,11 +21,11 @@ PROTOBUF_GO_VERSION ?= 1.33.0
GRPC_GO_VERSION ?= 1.3.0
GRPC_GATEWAY_VERSION ?= 2.19.1
VTPROTOBUF_VERSION ?= 0.6.0
GOIMPORTS_VERSION ?= 0.21.0
DEEPCOPY_VERSION ?= v0.5.6
GOLANGCILINT_VERSION ?= v1.56.2
GOLANGCILINT_VERSION ?= v1.58.0
GOFUMPT_VERSION ?= v0.6.0
GO_VERSION ?= 1.22.1
GOIMPORTS_VERSION ?= v0.19.0
GO_VERSION ?= 1.22.3
GO_BUILDFLAGS ?=
GO_LDFLAGS ?=
CGO_ENABLED ?= 0
Expand Down Expand Up @@ -59,9 +62,9 @@ COMMON_ARGS += --build-arg=PROTOBUF_GO_VERSION="$(PROTOBUF_GO_VERSION)"
COMMON_ARGS += --build-arg=GRPC_GO_VERSION="$(GRPC_GO_VERSION)"
COMMON_ARGS += --build-arg=GRPC_GATEWAY_VERSION="$(GRPC_GATEWAY_VERSION)"
COMMON_ARGS += --build-arg=VTPROTOBUF_VERSION="$(VTPROTOBUF_VERSION)"
COMMON_ARGS += --build-arg=GOIMPORTS_VERSION="$(GOIMPORTS_VERSION)"
COMMON_ARGS += --build-arg=DEEPCOPY_VERSION="$(DEEPCOPY_VERSION)"
COMMON_ARGS += --build-arg=GOLANGCILINT_VERSION="$(GOLANGCILINT_VERSION)"
COMMON_ARGS += --build-arg=GOIMPORTS_VERSION="$(GOIMPORTS_VERSION)"
COMMON_ARGS += --build-arg=GOFUMPT_VERSION="$(GOFUMPT_VERSION)"
COMMON_ARGS += --build-arg=TESTPKGS="$(TESTPKGS)"
TOOLCHAIN ?= docker.io/golang:1.22-alpine
Expand Down Expand Up @@ -110,7 +113,7 @@ If you already have a compatible builder instance, you may use that instead.
## Artifacts

All artifacts will be output to ./$(ARTIFACTS). Images will be tagged with the
registry "$(REGISTRY)", username "$(USERNAME)", and a dynamic tag (e.g. $(IMAGE):$(TAG)).
registry "$(REGISTRY)", username "$(USERNAME)", and a dynamic tag (e.g. $(IMAGE):$(IMAGE_TAG)).
The registry and username can be overridden by exporting REGISTRY, and USERNAME
respectively.

Expand All @@ -130,6 +133,9 @@ endif

all: unit-tests lint

$(ARTIFACTS): ## Creates artifacts directory.
@mkdir -p $(ARTIFACTS)

.PHONY: clean
clean: ## Cleans up all artifacts.
@rm -rf $(ARTIFACTS)
Expand Down Expand Up @@ -157,9 +163,6 @@ fmt: ## Formats the source code
lint-govulncheck: ## Runs govulncheck linter.
@$(MAKE) target-$@

lint-goimports: ## Runs goimports linter.
@$(MAKE) target-$@

.PHONY: base
base: ## Prepare base toolchain
@$(MAKE) target-$@
Expand All @@ -172,16 +175,12 @@ unit-tests: ## Performs unit tests
unit-tests-race: ## Performs unit tests with race detection enabled.
@$(MAKE) target-$@

.PHONY: coverage
coverage: ## Upload coverage data to codecov.io.
bash -c "bash <(curl -s https://codecov.io/bash) -f $(ARTIFACTS)/coverage-unit-tests.txt -X fix"

.PHONY: lint-markdown
lint-markdown: ## Runs markdownlint.
@$(MAKE) target-$@

.PHONY: lint
lint: lint-golangci-lint lint-gofumpt lint-govulncheck lint-goimports lint-markdown ## Run all linters for the project.
lint: lint-golangci-lint lint-gofumpt lint-govulncheck lint-markdown ## Run all linters for the project.

.PHONY: rekres
rekres:
Expand All @@ -194,8 +193,7 @@ help: ## This help menu.
@grep -E '^[a-zA-Z%_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

.PHONY: release-notes
release-notes:
mkdir -p $(ARTIFACTS)
release-notes: $(ARTIFACTS)
@ARTIFACTS=$(ARTIFACTS) ./hack/release.sh $@ $(ARTIFACTS)/RELEASE_NOTES.md $(TAG)

.PHONY: conformance
Expand Down
Loading

0 comments on commit 8485864

Please sign in to comment.