Skip to content

Commit f98f20f

Browse files
levbclaudeValentaTomas
authored
perf: use value BuildMap slices instead of pointer slices (#2319)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: ValentaTomas <valenta.and.thomas@gmail.com>
1 parent d355b84 commit f98f20f

8 files changed

Lines changed: 67 additions & 89 deletions

File tree

packages/orchestrator/cmd/show-build-diff/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func main() {
116116
fmt.Printf("Storage path %s\n", diffSource)
117117
fmt.Printf("========\n")
118118

119-
onlyDiffMappings := make([]*header.BuildMap, 0)
119+
onlyDiffMappings := make([]header.BuildMap, 0)
120120

121121
for _, mapping := range diffHeader.Mapping {
122122
if mapping.BuildId == diffHeader.Metadata.BuildId {

packages/orchestrator/pkg/sandbox/nbd/testutils/zero_device.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func NewZeroDevice(size int64, blockSize int64) (*ZeroDevice, error) {
2424
uint64(blockSize),
2525
uint64(size),
2626
),
27-
[]*header.BuildMap{
27+
[]header.BuildMap{
2828
{
2929
Offset: 0,
3030
Length: uint64(size),

packages/shared/pkg/storage/header/header.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ const NormalizeFixVersion = 3
1515

1616
type Header struct {
1717
Metadata *Metadata
18-
Mapping []*BuildMap
18+
Mapping []BuildMap
1919
}
2020

21-
func NewHeader(metadata *Metadata, mapping []*BuildMap) (*Header, error) {
21+
func NewHeader(metadata *Metadata, mapping []BuildMap) (*Header, error) {
2222
if metadata.BlockSize == 0 {
2323
return nil, fmt.Errorf("block size cannot be zero")
2424
}
2525

2626
if len(mapping) == 0 {
27-
mapping = []*BuildMap{{
27+
mapping = []BuildMap{{
2828
Offset: 0,
2929
Length: metadata.Size,
3030
BuildId: metadata.BuildId,
@@ -69,7 +69,6 @@ func (t *Header) GetShiftedMapping(ctx context.Context, offset int64) (mappedOff
6969
return mappedOffset, mappedLength, buildID, nil
7070
}
7171

72-
// TODO: Maybe we can optimize mapping by automatically assuming the mapping is uuid.Nil if we don't find it + stopping storing the nil mapping.
7372
func (t *Header) getMapping(ctx context.Context, offset int64) (*BuildMap, int64, error) {
7473
if offset < 0 || offset >= int64(t.Metadata.Size) {
7574
if t.IsNormalizeFixApplied() {
@@ -102,7 +101,7 @@ func (t *Header) getMapping(ctx context.Context, offset int64) (*BuildMap, int64
102101
return nil, 0, fmt.Errorf("no source found for offset %d", offset)
103102
}
104103

105-
mapping := t.Mapping[i-1]
104+
mapping := &t.Mapping[i-1]
106105
shift := offset - int64(mapping.Offset)
107106

108107
// Verify that the offset falls within this mapping's range

packages/shared/pkg/storage/header/inspect.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
// startBlock-endBlock [offset, offset+length) := [buildStorageOffset, buildStorageOffset+length) ⊂ buildId, length in bytes
1313
//
1414
// It is used for debugging and visualization.
15-
func (mapping *BuildMap) Format(blockSize uint64) string {
15+
func (mapping BuildMap) Format(blockSize uint64) string {
1616
rangeMessage := fmt.Sprintf("%d-%d", mapping.Offset/blockSize, (mapping.Offset+mapping.Length)/blockSize)
1717

1818
return fmt.Sprintf(
@@ -30,7 +30,7 @@ const (
3030
)
3131

3232
// Layers returns a map of buildIds that are present in the mappings.
33-
func Layers(mappings []*BuildMap) *map[uuid.UUID]struct{} {
33+
func Layers(mappings []BuildMap) *map[uuid.UUID]struct{} {
3434
layers := make(map[uuid.UUID]struct{})
3535

3636
for _, mapping := range mappings {
@@ -44,7 +44,7 @@ func Layers(mappings []*BuildMap) *map[uuid.UUID]struct{} {
4444
// It is used for debugging and visualization.
4545
//
4646
// You can pass maps to visualize different groups of buildIds.
47-
func Visualize(mappings []*BuildMap, size, blockSize, cols uint64, bottomGroup, topGroup *map[uuid.UUID]struct{}) string {
47+
func Visualize(mappings []BuildMap, size, blockSize, cols uint64, bottomGroup, topGroup *map[uuid.UUID]struct{}) string {
4848
output := make([]rune, size/blockSize)
4949

5050
for outputIdx := range output {
@@ -85,7 +85,7 @@ func Visualize(mappings []*BuildMap, size, blockSize, cols uint64, bottomGroup,
8585
//
8686
// It checks if the mappings are contiguous and if the length of each mapping is a multiple of the block size.
8787
// It also checks if the mappings cover the whole size.
88-
func ValidateMappings(mappings []*BuildMap, size, blockSize uint64) error {
88+
func ValidateMappings(mappings []BuildMap, size, blockSize uint64) error {
8989
var currentOffset uint64
9090

9191
for _, mapping := range mappings {
@@ -111,11 +111,11 @@ func ValidateMappings(mappings []*BuildMap, size, blockSize uint64) error {
111111
return nil
112112
}
113113

114-
func (mapping *BuildMap) Equal(other *BuildMap) bool {
114+
func (mapping BuildMap) Equal(other BuildMap) bool {
115115
return mapping.Offset == other.Offset && mapping.Length == other.Length && mapping.BuildId == other.BuildId
116116
}
117117

118-
func Equal(a, b []*BuildMap) bool {
118+
func Equal(a, b []BuildMap) bool {
119119
if len(a) != len(b) {
120120
return false
121121
}

packages/shared/pkg/storage/header/mapping.go

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,12 @@ type BuildMap struct {
1919
BuildStorageOffset uint64
2020
}
2121

22-
func (mapping *BuildMap) Copy() *BuildMap {
23-
return &BuildMap{
24-
Offset: mapping.Offset,
25-
Length: mapping.Length,
26-
BuildId: mapping.BuildId,
27-
BuildStorageOffset: mapping.BuildStorageOffset,
28-
}
29-
}
30-
3122
func CreateMapping(
3223
buildId *uuid.UUID,
3324
dirty *bitset.BitSet,
3425
blockSize int64,
35-
) []*BuildMap {
36-
var mappings []*BuildMap
26+
) []BuildMap {
27+
var mappings []BuildMap
3728

3829
var startBlock uint
3930
var blockLength uint
@@ -47,7 +38,7 @@ func CreateMapping(
4738
}
4839

4940
if blockLength > 0 {
50-
m := &BuildMap{
41+
m := BuildMap{
5142
Offset: uint64(startBlock) * uint64(blockSize),
5243
BuildId: *buildId,
5344
Length: uint64(blockLength) * uint64(blockSize),
@@ -64,7 +55,7 @@ func CreateMapping(
6455
}
6556

6657
if blockLength > 0 {
67-
mappings = append(mappings, &BuildMap{
58+
mappings = append(mappings, BuildMap{
6859
Offset: uint64(startBlock) * uint64(blockSize),
6960
BuildId: *buildId,
7061
Length: uint64(blockLength) * uint64(blockSize),
@@ -82,26 +73,26 @@ func CreateMapping(
8273
//
8374
// It returns a new set of mappings that covers the whole size.
8475
func MergeMappings(
85-
baseMapping []*BuildMap,
86-
diffMapping []*BuildMap,
87-
) []*BuildMap {
76+
baseMapping []BuildMap,
77+
diffMapping []BuildMap,
78+
) []BuildMap {
8879
if len(diffMapping) == 0 {
8980
return baseMapping
9081
}
9182

92-
baseMappingCopy := make([]*BuildMap, len(baseMapping))
83+
baseMappingCopy := make([]BuildMap, len(baseMapping))
9384

9485
copy(baseMappingCopy, baseMapping)
9586

9687
baseMapping = baseMappingCopy
9788

98-
mappings := make([]*BuildMap, 0)
89+
mappings := make([]BuildMap, 0)
9990

10091
var baseIdx int
10192
var diffIdx int
10293

10394
for baseIdx < len(baseMapping) && diffIdx < len(diffMapping) {
104-
base := baseMapping[baseIdx]
95+
base := &baseMapping[baseIdx]
10596
diff := diffMapping[diffIdx]
10697

10798
if base.Length == 0 {
@@ -119,7 +110,7 @@ func MergeMappings(
119110
// base is before diff and there is no overlap
120111
// add base to the result, because it will not be overlapping by any diff
121112
if base.Offset+base.Length <= diff.Offset {
122-
mappings = append(mappings, base)
113+
mappings = append(mappings, *base)
123114

124115
baseIdx++
125116

@@ -153,15 +144,13 @@ func MergeMappings(
153144
leftBaseLength := int64(diff.Offset) - int64(base.Offset)
154145

155146
if leftBaseLength > 0 {
156-
leftBase := &BuildMap{
147+
mappings = append(mappings, BuildMap{
157148
Offset: base.Offset,
158149
Length: uint64(leftBaseLength),
159150
BuildId: base.BuildId,
160151
// the build storage offset is the same as the base mapping
161152
BuildStorageOffset: base.BuildStorageOffset,
162-
}
163-
164-
mappings = append(mappings, leftBase)
153+
})
165154
}
166155

167156
mappings = append(mappings, diff)
@@ -172,14 +161,12 @@ func MergeMappings(
172161
rightBaseLength := int64(base.Length) - rightBaseShift
173162

174163
if rightBaseLength > 0 {
175-
rightBase := &BuildMap{
164+
baseMapping[baseIdx] = BuildMap{
176165
Offset: base.Offset + uint64(rightBaseShift),
177166
Length: uint64(rightBaseLength),
178167
BuildId: base.BuildId,
179168
BuildStorageOffset: base.BuildStorageOffset + uint64(rightBaseShift),
180169
}
181-
182-
baseMapping[baseIdx] = rightBase
183170
} else {
184171
baseIdx++
185172
}
@@ -199,14 +186,12 @@ func MergeMappings(
199186
rightBaseLength := int64(base.Length) - rightBaseShift
200187

201188
if rightBaseLength > 0 {
202-
rightBase := &BuildMap{
189+
baseMapping[baseIdx] = BuildMap{
203190
Offset: base.Offset + uint64(rightBaseShift),
204191
Length: uint64(rightBaseLength),
205192
BuildId: base.BuildId,
206193
BuildStorageOffset: base.BuildStorageOffset + uint64(rightBaseShift),
207194
}
208-
209-
baseMapping[baseIdx] = rightBase
210195
} else {
211196
baseIdx++
212197
}
@@ -220,14 +205,12 @@ func MergeMappings(
220205
leftBaseLength := int64(diff.Offset) - int64(base.Offset)
221206

222207
if leftBaseLength > 0 {
223-
leftBase := &BuildMap{
208+
mappings = append(mappings, BuildMap{
224209
Offset: base.Offset,
225210
Length: uint64(leftBaseLength),
226211
BuildId: base.BuildId,
227212
BuildStorageOffset: base.BuildStorageOffset,
228-
}
229-
230-
mappings = append(mappings, leftBase)
213+
})
231214
}
232215

233216
baseIdx++
@@ -245,29 +228,25 @@ func MergeMappings(
245228
}
246229

247230
// NormalizeMappings joins adjacent mappings that have the same buildId.
248-
func NormalizeMappings(mappings []*BuildMap) []*BuildMap {
231+
func NormalizeMappings(mappings []BuildMap) []BuildMap {
249232
if len(mappings) == 0 {
250233
return nil
251234
}
252235

253-
result := make([]*BuildMap, 0, len(mappings))
236+
result := make([]BuildMap, 0, len(mappings))
254237

255-
// Start with a copy of the first mapping
256-
current := mappings[0].Copy()
238+
current := mappings[0]
257239

258240
for i := 1; i < len(mappings); i++ {
259241
mp := mappings[i]
260-
if mp.BuildId != current.BuildId {
261-
// BuildId changed, add the current map to results and start a new one
262-
result = append(result, current)
263-
current = mp.Copy() // New copy
264-
} else {
265-
// Same BuildId, just add the length
242+
if mp.BuildId == current.BuildId {
266243
current.Length += mp.Length
244+
} else {
245+
result = append(result, current)
246+
current = mp
267247
}
268248
}
269249

270-
// Add the last mapping
271250
result = append(result, current)
272251

273252
return result

0 commit comments

Comments
 (0)