Skip to content

Commit 015a398

Browse files
committed
fix: replace static buffer allocation on growth
Use `slices.Grow` and follow whatever algorithm it uses internally to pick the next size of the buffer. So the growth might not be exactly what we asked for, but it should be optimal from Go runtime point of view. Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
1 parent ed8685e commit 015a398

File tree

8 files changed

+82
-39
lines changed

8 files changed

+82
-39
lines changed

.github/renovate.json

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
3+
"description": "THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.",
4+
"prHeader": "Update Request | Renovate Bot",
5+
"extends": [
6+
":dependencyDashboard",
7+
":gitSignOff",
8+
":semanticCommitScopeDisabled",
9+
"schedule:earlyMondays"
10+
],
11+
"packageRules": [
12+
{
13+
"groupName": "dependencies",
14+
"matchUpdateTypes": [
15+
"major",
16+
"minor",
17+
"patch",
18+
"pin",
19+
"digest"
20+
]
21+
},
22+
{
23+
"enabled": false,
24+
"matchFileNames": [
25+
"Dockerfile"
26+
]
27+
},
28+
{
29+
"enabled": false,
30+
"matchFileNames": [
31+
".github/workflows/*.yaml"
32+
]
33+
}
34+
],
35+
"separateMajorMinor": false
36+
}

.golangci.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
22
#
3-
# Generated on 2024-11-26T17:07:18Z by kres 232fe63.
3+
# Generated on 2025-01-30T12:16:36Z by kres 987bf4d.
44

55
# options for analysis running
66
run:
@@ -17,7 +17,6 @@ output:
1717
path: stdout
1818
print-issued-lines: true
1919
print-linter-name: true
20-
uniq-by-line: true
2120
path-prefix: ""
2221

2322
# all available settings of specific linters
@@ -134,6 +133,7 @@ linters:
134133
- perfsprint # complains about us using fmt.Sprintf in non-performance critical code, updating just kres took too long
135134
- goimports # same as gci
136135
- musttag # seems to be broken - goes into imported libraries and reports issues there
136+
- exportloopref # WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.
137137

138138
issues:
139139
exclude: [ ]
@@ -143,6 +143,7 @@ issues:
143143
max-issues-per-linter: 10
144144
max-same-issues: 3
145145
new: false
146+
uniq-by-line: true
146147

147148
severity:
148149
default-severity: error

Dockerfile

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
1-
# syntax = docker/dockerfile-upstream:1.11.1-labs
1+
# syntax = docker/dockerfile-upstream:1.12.1-labs
22

33
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
44
#
5-
# Generated on 2024-11-26T17:07:18Z by kres 232fe63.
5+
# Generated on 2025-01-30T12:16:36Z by kres 987bf4d.
66

77
ARG TOOLCHAIN
88

99
# cleaned up specs and compiled versions
1010
FROM scratch AS generate
1111

1212
# runs markdownlint
13-
FROM docker.io/oven/bun:1.1.36-alpine AS lint-markdown
13+
FROM docker.io/oven/bun:1.1.43-alpine AS lint-markdown
1414
WORKDIR /src
15-
RUN bun i markdownlint-cli@0.43.0 sentences-per-line@0.2.1
15+
RUN bun i markdownlint-cli@0.43.0 sentences-per-line@0.3.0
1616
COPY .markdownlint.json .
1717
COPY ./README.md ./README.md
18-
RUN bunx markdownlint --ignore "CHANGELOG.md" --ignore "**/node_modules/**" --ignore '**/hack/chglog/**' --rules node_modules/sentences-per-line/index.js .
18+
RUN bunx markdownlint --ignore "CHANGELOG.md" --ignore "**/node_modules/**" --ignore '**/hack/chglog/**' --rules sentences-per-line .
1919

2020
# base toolchain image
2121
FROM --platform=${BUILDPLATFORM} ${TOOLCHAIN} AS toolchain

Makefile

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
22
#
3-
# Generated on 2024-11-26T17:07:18Z by kres 232fe63.
3+
# Generated on 2025-01-30T12:16:36Z by kres 987bf4d.
44

55
# common variables
66

@@ -17,15 +17,15 @@ WITH_RACE ?= false
1717
REGISTRY ?= ghcr.io
1818
USERNAME ?= siderolabs
1919
REGISTRY_AND_USERNAME ?= $(REGISTRY)/$(USERNAME)
20-
PROTOBUF_GO_VERSION ?= 1.35.2
20+
PROTOBUF_GO_VERSION ?= 1.36.2
2121
GRPC_GO_VERSION ?= 1.5.1
22-
GRPC_GATEWAY_VERSION ?= 2.24.0
22+
GRPC_GATEWAY_VERSION ?= 2.25.1
2323
VTPROTOBUF_VERSION ?= 0.6.0
24-
GOIMPORTS_VERSION ?= 0.27.0
24+
GOIMPORTS_VERSION ?= 0.29.0
2525
DEEPCOPY_VERSION ?= v0.5.6
26-
GOLANGCILINT_VERSION ?= v1.62.0
26+
GOLANGCILINT_VERSION ?= v1.63.4
2727
GOFUMPT_VERSION ?= v0.7.0
28-
GO_VERSION ?= 1.23.3
28+
GO_VERSION ?= 1.23.5
2929
GO_BUILDFLAGS ?=
3030
GO_LDFLAGS ?=
3131
CGO_ENABLED ?= 0
@@ -41,13 +41,13 @@ PLATFORM ?= linux/amd64
4141
PROGRESS ?= auto
4242
PUSH ?= false
4343
CI_ARGS ?=
44-
BUILDKIT_MULTI_PLATFORM ?= 1
44+
BUILDKIT_MULTI_PLATFORM ?=
4545
COMMON_ARGS = --file=Dockerfile
4646
COMMON_ARGS += --provenance=false
4747
COMMON_ARGS += --progress=$(PROGRESS)
4848
COMMON_ARGS += --platform=$(PLATFORM)
49-
COMMON_ARGS += --push=$(PUSH)
5049
COMMON_ARGS += --build-arg=BUILDKIT_MULTI_PLATFORM=$(BUILDKIT_MULTI_PLATFORM)
50+
COMMON_ARGS += --push=$(PUSH)
5151
COMMON_ARGS += --build-arg=ARTIFACTS="$(ARTIFACTS)"
5252
COMMON_ARGS += --build-arg=SHA="$(SHA)"
5353
COMMON_ARGS += --build-arg=TAG="$(TAG)"
@@ -145,14 +145,17 @@ clean: ## Cleans up all artifacts.
145145
target-%: ## Builds the specified target defined in the Dockerfile. The build result will only remain in the build cache.
146146
@$(BUILD) --target=$* $(COMMON_ARGS) $(TARGET_ARGS) $(CI_ARGS) .
147147

148+
registry-%: ## Builds the specified target defined in the Dockerfile and the output is an image. The image is pushed to the registry if PUSH=true.
149+
@$(MAKE) target-$* TARGET_ARGS="--tag=$(REGISTRY)/$(USERNAME)/$(IMAGE_NAME):$(IMAGE_TAG)" BUILDKIT_MULTI_PLATFORM=1
150+
148151
local-%: ## Builds the specified target defined in the Dockerfile using the local output type. The build result will be output to the specified local destination.
149152
@$(MAKE) target-$* TARGET_ARGS="--output=type=local,dest=$(DEST) $(TARGET_ARGS)"
150153
@PLATFORM=$(PLATFORM) DEST=$(DEST) bash -c '\
151154
for platform in $$(tr "," "\n" <<< "$$PLATFORM"); do \
152-
echo $$platform; \
153155
directory="$${platform//\//_}"; \
154156
if [[ -d "$$DEST/$$directory" ]]; then \
155-
mv "$$DEST/$$directory/"* $$DEST; \
157+
echo $$platform; \
158+
mv -f "$$DEST/$$directory/"* $$DEST; \
156159
rmdir "$$DEST/$$directory/"; \
157160
fi; \
158161
done'

circular.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (buf *Buffer) Write(p []byte) (int, error) {
112112
defer buf.mu.Unlock()
113113

114114
if buf.off < int64(buf.opt.MaxCapacity) {
115-
if buf.off+int64(l) > int64(cap(buf.data)) && cap(buf.data) < buf.opt.MaxCapacity {
115+
if buf.off+int64(l) > int64(buf.capacity()) && buf.capacity() < buf.opt.MaxCapacity {
116116
// grow buffer to ensure write fits, but limit with max capacity
117117
size := cap(buf.data) * 2
118118
for size < int(buf.off)+l {
@@ -123,9 +123,9 @@ func (buf *Buffer) Write(p []byte) (int, error) {
123123
size = buf.opt.MaxCapacity
124124
}
125125

126-
data := make([]byte, size)
127-
copy(data, buf.data)
128-
buf.data = data
126+
// grow and adjust length to the desired capacity
127+
data := slices.Grow(buf.data, size)
128+
buf.data = data[:min(cap(data), buf.opt.MaxCapacity)]
129129
}
130130
}
131131

@@ -193,12 +193,20 @@ func (buf *Buffer) Write(p []byte) (int, error) {
193193
return n, nil
194194
}
195195

196+
// capacity returns the current buffer capacity.
197+
func (buf *Buffer) capacity() int {
198+
// we return length here, not capacity, as the buffer might be larger due to the way slices.Grow works
199+
//
200+
// this is the useable length of the buffer, we might grow it (and actual capacity might or might not grow, leave it to slices.Grow)
201+
return len(buf.data)
202+
}
203+
196204
// Capacity returns number of bytes allocated for the buffer.
197205
func (buf *Buffer) Capacity() int {
198206
buf.mu.Lock()
199207
defer buf.mu.Unlock()
200208

201-
return cap(buf.data)
209+
return buf.capacity()
202210
}
203211

204212
// MaxCapacity returns maximum number of (decompressed) bytes (including compressed chunks) that can be stored in the buffer.
@@ -225,7 +233,7 @@ func (buf *Buffer) TotalCompressedSize() int64 {
225233
size += int64(len(c.compressed))
226234
}
227235

228-
return size + int64(cap(buf.data))
236+
return size + int64(buf.capacity())
229237
}
230238

231239
// TotalSize reports overall number of bytes available for reading in the buffer.
@@ -237,11 +245,7 @@ func (buf *Buffer) TotalSize() int64 {
237245
defer buf.mu.Unlock()
238246

239247
if len(buf.chunks) == 0 {
240-
if buf.off < int64(cap(buf.data)) {
241-
return buf.off
242-
}
243-
244-
return int64(cap(buf.data))
248+
return min(int64(buf.capacity()), buf.off)
245249
}
246250

247251
return buf.off - buf.chunks[0].startOffset

circular_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ func TestWrites(t *testing.T) {
8787
req.NoError(err)
8888
req.Equal(5000, n)
8989

90-
req.Equal(8192, buf.Capacity())
90+
// capacity might grow to a number specific to the way `slices.Grow` works,
91+
// but it should be within reasonable bounds
92+
req.GreaterOrEqual(buf.Capacity(), 8192)
93+
req.LessOrEqual(buf.Capacity(), 16384)
94+
9195
req.EqualValues(6100, buf.Offset())
9296

9397
for i := range 20 {

persistence.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"go.uber.org/zap"
2020
)
2121

22-
//nolint:gocognit
2322
func (buf *Buffer) load() error {
2423
if buf.opt.PersistenceOptions.ChunkPath == "" {
2524
// persistence is disabled
@@ -97,18 +96,14 @@ func (buf *Buffer) load() error {
9796
if err != nil {
9897
buf.opt.Logger.Error("failed to decompress zero chunk, skipping", zap.String("path", chunkPath.path), zap.Error(err))
9998

100-
buf.data = buf.data[:cap(buf.data)]
99+
buf.data = buf.data[:min(cap(buf.data), buf.opt.MaxCapacity)]
101100

102101
continue
103102
}
104103

105104
buf.off = int64(len(buf.data))
106105

107-
if cap(buf.data) > buf.opt.MaxCapacity {
108-
buf.data = buf.data[:buf.opt.MaxCapacity:buf.opt.MaxCapacity]
109-
} else {
110-
buf.data = buf.data[:cap(buf.data)]
111-
}
106+
buf.data = buf.data[:min(cap(buf.data), buf.opt.MaxCapacity)]
112107
} else {
113108
decompressedSize, err := buf.opt.Compressor.DecompressedSize(data)
114109
if err != nil {
@@ -276,7 +271,7 @@ func (buf *Buffer) persistCurrentChunk(currentOffset, lastPersistedOffset int64,
276271
}
277272

278273
buf.mu.Lock()
279-
data := slices.Clone(buf.data[:currentOffset%int64(cap(buf.data))])
274+
data := slices.Clone(buf.data[:currentOffset%int64(buf.capacity())])
280275
buf.mu.Unlock()
281276

282277
compressed, err := buf.opt.Compressor.Compress(data, nil)

reader.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,11 @@ func (r *Reader) Read(p []byte) (n int, err error) {
126126
n = int(r.endOff - r.off)
127127
}
128128

129-
n = min(min(n, len(p)), cap(r.buf.data))
129+
n = min(min(n, len(p)), r.buf.capacity())
130130

131131
i := int(r.off % int64(r.buf.opt.MaxCapacity))
132132

133-
if l := cap(r.buf.data) - i; l < n {
133+
if l := r.buf.capacity() - i; l < n {
134134
copy(p, r.buf.data[i:])
135135
copy(p[l:], r.buf.data[:n-l])
136136
} else {

0 commit comments

Comments
 (0)