Skip to content

Commit e072aa0

Browse files
authored
refactor: optimize code with modern Go patterns and pre-allocation (#3576)
This commit modernizes code across multiple packages by: - Using Go 1.18+ features (slices.IndexFunc, strings.Cut) - Pre-allocating slices and maps with known capacity - Consolidating defensive checks and improving code clarity - Fixing test data and build tag issues CLI client improvements: - Pre-allocate slices in search functions and service methods - Replace strings.Split with strings.Cut for username:password parsing - Use range-based iteration instead of manual index loops Search extension optimizations: - Cache sort functions in pagination modules - Pre-allocate page buffers and maps - Consolidate defensive checks in filterBaseImages/filterDerivedImages - Fix image bas and derived logic allowing out of sequence layers for base images - Fix image pagination reporting images groupped by repos when sorted by update time - Remove duplicate resolver_test.go file Monitoring extension: - Replace manual loops with slices.IndexFunc - Pre-allocate bucketsFloat2String map Sync extension: - Pre-allocate slice in parseRegistryURLs Test utilities: - Fix build tags in oci_layout.go Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
1 parent da42685 commit e072aa0

17 files changed

Lines changed: 1039 additions & 431 deletions

File tree

pkg/cli/client/client.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,8 @@ func fetchManifestStruct(ctx context.Context, repo, manifestReference string, se
445445
imageSize += manifestResp.Config.Size
446446
imageSize += manifestSize
447447

448-
layers := []common.LayerSummary{}
448+
// Pre-allocate slice with known capacity
449+
layers := make([]common.LayerSummary, 0, len(manifestResp.Layers))
449450

450451
for _, entry := range manifestResp.Layers {
451452
imageSize += entry.Size

pkg/cli/client/search_functions.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ func SearchAllImagesGQL(config SearchConfig) error {
5151
return err
5252
}
5353

54-
imageListData := []imageStruct{}
54+
// Pre-allocate slice with known capacity
55+
imageListData := make([]imageStruct, 0, len(imageList.Results))
5556

5657
for _, image := range imageList.Results {
5758
imageListData = append(imageListData, imageStruct(image))
@@ -103,7 +104,8 @@ func SearchImageByNameGQL(config SearchConfig, imageName string) error {
103104
return err
104105
}
105106

106-
imageListData := []imageStruct{}
107+
// Pre-allocate slice with known capacity (may be filtered, but worst case is all results)
108+
imageListData := make([]imageStruct, 0, len(imageList.Results))
107109

108110
for _, image := range imageList.Results {
109111
if tag == "" || image.Tag == tag {
@@ -152,7 +154,8 @@ func SearchDerivedImageListGQL(config SearchConfig, derivedImage string) error {
152154
return err
153155
}
154156

155-
imageListData := []imageStruct{}
157+
// Pre-allocate slice with known capacity
158+
imageListData := make([]imageStruct, 0, len(imageList.DerivedImageList.Results))
156159

157160
for _, image := range imageList.DerivedImageList.Results {
158161
imageListData = append(imageListData, imageStruct(image))
@@ -173,7 +176,8 @@ func SearchBaseImageListGQL(config SearchConfig, baseImage string) error {
173176
return err
174177
}
175178

176-
imageListData := []imageStruct{}
179+
// Pre-allocate slice with known capacity
180+
imageListData := make([]imageStruct, 0, len(imageList.BaseImageList.Results))
177181

178182
for _, image := range imageList.BaseImageList.Results {
179183
imageListData = append(imageListData, imageStruct(image))
@@ -193,7 +197,8 @@ func SearchImagesForDigestGQL(config SearchConfig, digest string) error {
193197
return err
194198
}
195199

196-
imageListData := []imageStruct{}
200+
// Pre-allocate slice with known capacity
201+
imageListData := make([]imageStruct, 0, len(imageList.Results))
197202

198203
for _, image := range imageList.Results {
199204
imageListData = append(imageListData, imageStruct(image))
@@ -344,7 +349,8 @@ func SearchImagesByCVEIDGQL(config SearchConfig, repo, cveid string) error {
344349
return err
345350
}
346351

347-
imageListData := []imageStruct{}
352+
// Pre-allocate slice with known capacity
353+
imageListData := make([]imageStruct, 0, len(imageList.Results))
348354

349355
for _, image := range imageList.Results {
350356
imageListData = append(imageListData, imageStruct(image))
@@ -403,13 +409,14 @@ func GlobalSearchGQL(config SearchConfig, query string) error {
403409
return err
404410
}
405411

406-
imagesList := []imageStruct{}
412+
// Pre-allocate slices with known capacity
413+
imagesList := make([]imageStruct, 0, len(globalSearchResult.Images))
407414

408415
for _, image := range globalSearchResult.Images {
409416
imagesList = append(imagesList, imageStruct(image))
410417
}
411418

412-
reposList := []repoStruct{}
419+
reposList := make([]repoStruct, 0, len(globalSearchResult.Repos))
413420

414421
for _, repo := range globalSearchResult.Repos {
415422
reposList = append(reposList, repoStruct(repo))

pkg/cli/client/service.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,9 @@ func (service searchService) getTagsForCVEGQL(ctx context.Context, config Search
403403
return result, nil
404404
}
405405

406+
// Pre-allocate filtered results slice with estimated capacity
406407
filteredResults := &common.ImagesForCve{}
408+
filteredResults.Results = make([]common.ImageSummary, 0, len(result.Results))
407409

408410
for _, image := range result.Results {
409411
if image.RepoName == repo {
@@ -476,7 +478,8 @@ func (service searchService) getReferrers(ctx context.Context, config SearchConf
476478
return referrersResult{}, err
477479
}
478480

479-
referrersList := referrersResult{}
481+
// Pre-allocate referrers list with known capacity
482+
referrersList := make(referrersResult, 0, len(referrerResp.Manifests))
480483

481484
for _, referrer := range referrerResp.Manifests {
482485
referrersList = append(referrersList, common.Referrer{
@@ -954,7 +957,8 @@ func (ref referrersResult) stringPlainText(maxArtifactTypeLen int) (string, erro
954957
var builder strings.Builder
955958

956959
maxDigestWidth := digestWidth
957-
rows := [][]string{}
960+
// Pre-allocate rows slice with known capacity
961+
rows := make([][]string, 0, len(ref))
958962

959963
for _, referrer := range ref {
960964
artifactType := ellipsize(referrer.ArtifactType, maxArtifactTypeLen, ellipsis)
@@ -1390,12 +1394,14 @@ func (service searchService) getRepos(ctx context.Context, config SearchConfig,
13901394
fmt.Fprintln(config.ResultWriter, "\nREPOSITORY NAME")
13911395

13921396
if config.SortBy == SortByAlphabeticAsc {
1393-
for i := 0; i < len(catalog.Repositories); i++ {
1394-
fmt.Fprintln(config.ResultWriter, catalog.Repositories[i])
1397+
for _, repo := range catalog.Repositories {
1398+
fmt.Fprintln(config.ResultWriter, repo)
13951399
}
13961400
} else {
1397-
for i := len(catalog.Repositories) - 1; i >= 0; i-- {
1398-
fmt.Fprintln(config.ResultWriter, catalog.Repositories[i])
1401+
// Iterate in reverse order
1402+
repos := catalog.Repositories
1403+
for i := len(repos) - 1; i >= 0; i-- {
1404+
fmt.Fprintln(config.ResultWriter, repos[i])
13991405
}
14001406
}
14011407
}

pkg/cli/client/utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ func collectResults(config SearchConfig, wg *sync.WaitGroup, imageErr chan strin
9393

9494
func getUsernameAndPassword(user string) (string, string) {
9595
if strings.Contains(user, ":") {
96-
split := strings.Split(user, ":")
96+
username, password, _ := strings.Cut(user, ":")
9797

98-
return split[0], split[1]
98+
return username, password
9999
}
100100

101101
return "", ""

pkg/extensions/monitoring/minimal.go

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"math"
99
"path"
10+
"slices"
1011
"strconv"
1112
"sync"
1213
"time"
@@ -236,9 +237,12 @@ func NewMetricsServer(enabled bool, log log.Logger) MetricServer {
236237
Histograms: make([]*HistogramValue, 0),
237238
}
238239
// convert to a map for returning easily the string corresponding to a bucket
239-
bucketsFloat2String := map[float64]string{}
240+
// Pre-allocate with known size: default buckets + storage latency buckets
241+
defaultBuckets := GetDefaultBuckets()
242+
storageBuckets := GetStorageLatencyBuckets()
243+
bucketsFloat2String := make(map[float64]string, len(defaultBuckets)+len(storageBuckets))
240244

241-
for _, fvalue := range append(GetDefaultBuckets(), GetStorageLatencyBuckets()...) {
245+
for _, fvalue := range append(defaultBuckets, storageBuckets...) {
242246
if fvalue == math.MaxFloat64 {
243247
bucketsFloat2String[fvalue] = "+Inf"
244248
} else {
@@ -314,54 +318,38 @@ func isMetricMatch(lValues, metricValues []string) bool {
314318

315319
// returns {-1, false} in case metric was not found in the slice.
316320
func findCounterValueIndex(metricSlice []*CounterValue, name string, labelValues []string) (int, bool) {
317-
for i, m := range metricSlice {
318-
if m.Name == name {
319-
if isMetricMatch(labelValues, m.LabelValues) {
320-
return i, true
321-
}
322-
}
323-
}
321+
idx := slices.IndexFunc(metricSlice, func(m *CounterValue) bool {
322+
return m.Name == name && isMetricMatch(labelValues, m.LabelValues)
323+
})
324324

325-
return -1, false
325+
return idx, idx != -1
326326
}
327327

328328
// returns {-1, false} in case metric was not found in the slice.
329329
func findGaugeValueIndex(metricSlice []*GaugeValue, name string, labelValues []string) (int, bool) {
330-
for i, m := range metricSlice {
331-
if m.Name == name {
332-
if isMetricMatch(labelValues, m.LabelValues) {
333-
return i, true
334-
}
335-
}
336-
}
330+
idx := slices.IndexFunc(metricSlice, func(m *GaugeValue) bool {
331+
return m.Name == name && isMetricMatch(labelValues, m.LabelValues)
332+
})
337333

338-
return -1, false
334+
return idx, idx != -1
339335
}
340336

341337
// returns {-1, false} in case metric was not found in the slice.
342338
func findSummaryValueIndex(metricSlice []*SummaryValue, name string, labelValues []string) (int, bool) {
343-
for i, m := range metricSlice {
344-
if m.Name == name {
345-
if isMetricMatch(labelValues, m.LabelValues) {
346-
return i, true
347-
}
348-
}
349-
}
339+
idx := slices.IndexFunc(metricSlice, func(m *SummaryValue) bool {
340+
return m.Name == name && isMetricMatch(labelValues, m.LabelValues)
341+
})
350342

351-
return -1, false
343+
return idx, idx != -1
352344
}
353345

354346
// returns {-1, false} in case metric was not found in the slice.
355347
func findHistogramValueIndex(metricSlice []*HistogramValue, name string, labelValues []string) (int, bool) {
356-
for i, m := range metricSlice {
357-
if m.Name == name {
358-
if isMetricMatch(labelValues, m.LabelValues) {
359-
return i, true
360-
}
361-
}
362-
}
348+
idx := slices.IndexFunc(metricSlice, func(m *HistogramValue) bool {
349+
return m.Name == name && isMetricMatch(labelValues, m.LabelValues)
350+
})
363351

364-
return -1, false
352+
return idx, idx != -1
365353
}
366354

367355
func (ms *metricServer) CounterInc(cv *CounterValue) {

pkg/extensions/search/convert/metadb.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ func getAnnotationsFromMap(annotationsMap map[string]string) []*gql_generated.An
7373
func getImageBlobsInfo(manifestDigest string, manifestSize int64, configDigest string, configSize int64,
7474
layers []ispec.Descriptor,
7575
) (int64, map[string]int64) {
76-
imageBlobsMap := map[string]int64{}
76+
// Pre-allocate map with known size: config + manifest + layers
77+
imageBlobsMap := make(map[string]int64, 2+len(layers))
7778
imageSize := int64(0)
7879

7980
// add config size

pkg/extensions/search/cve/cve.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package cveinfo
22

33
import (
44
"context"
5-
"sort"
5+
"slices"
66
"strings"
77
"time"
88

@@ -528,8 +528,15 @@ func (cveinfo BaseCveInfo) GetCVESummaryForImageMedia(ctx context.Context, repo,
528528
}
529529

530530
func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo {
531-
sort.Slice(allTags, func(i, j int) bool {
532-
return allTags[i].Timestamp.Before(allTags[j].Timestamp)
531+
slices.SortFunc(allTags, func(a, b cvemodel.TagInfo) int {
532+
if a.Timestamp.Before(b.Timestamp) {
533+
return -1
534+
}
535+
if a.Timestamp.Equal(b.Timestamp) {
536+
return 0
537+
}
538+
539+
return 1
533540
})
534541

535542
earliestVulnerable := vulnerableTags[0]
@@ -597,7 +604,10 @@ func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo
597604
}
598605

599606
// check if the current manifest doesn't have the vulnerability
600-
if !indexHasVulnerableManifest || !containsDescriptorInfo(vulnTagInfo.Manifests, manifestDesc) {
607+
if !indexHasVulnerableManifest ||
608+
!slices.ContainsFunc(vulnTagInfo.Manifests, func(di cvemodel.DescriptorInfo) bool {
609+
return di.Digest == manifestDesc.Digest
610+
}) {
601611
fixedManifests = append(fixedManifests, manifestDesc)
602612
}
603613
}
@@ -616,16 +626,6 @@ func GetFixedTags(allTags, vulnerableTags []cvemodel.TagInfo) []cvemodel.TagInfo
616626
return fixedTags
617627
}
618628

619-
func containsDescriptorInfo(slice []cvemodel.DescriptorInfo, descriptorInfo cvemodel.DescriptorInfo) bool {
620-
for _, di := range slice {
621-
if di.Digest == descriptorInfo.Digest {
622-
return true
623-
}
624-
}
625-
626-
return false
627-
}
628-
629629
func initCVESummaryFromCVEMap(cveMap map[string]cvemodel.CVE) cvemodel.ImageCVESummary {
630630
// Counters are initialized with 0 by default
631631
imageCVESummary := cvemodel.ImageCVESummary{

0 commit comments

Comments
 (0)