Refactor operations with maps

Removes the duplicate copy*Map function using the general function newMapFrom. Reduces the allocation of empty maps using the copyMapPrefferingNil function. This change may affect the behavior so that instead of an empty allocated map, a nil will be returned. Signed-off-by: Jan Rodák <[email protected]>
containers · Oct 9, 2024 · 1b0556c · 1b0556c
1 parent 624a0ab
commit 1b0556c
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 25 deletions.
diff --git a/containers.go b/containers.go
@@ -3,7 +3,6 @@ package storage
 import (
 	"errors"
 	"fmt"
-	"maps"
 	"os"
 	"path/filepath"
 	"slices"
@@ -169,12 +168,12 @@ func copyContainer(c *Container) *Container {
 		LayerID:        c.LayerID,
 		Metadata:       c.Metadata,
 		BigDataNames:   copySlicePreferringNil(c.BigDataNames),
-		BigDataSizes:   maps.Clone(c.BigDataSizes),
-		BigDataDigests: maps.Clone(c.BigDataDigests),
+		BigDataSizes:   copyMapPreferringNil(c.BigDataSizes),
+		BigDataDigests: copyMapPreferringNil(c.BigDataDigests),
 		Created:        c.Created,
 		UIDMap:         copySlicePreferringNil(c.UIDMap),
 		GIDMap:         copySlicePreferringNil(c.GIDMap),
-		Flags:          maps.Clone(c.Flags),
+		Flags:          copyMapPreferringNil(c.Flags),
 		volatileStore:  c.volatileStore,
 	}
 }
@@ -692,7 +691,7 @@ func (r *containerStore) create(id string, names []string, image, layer string,
 		BigDataSizes:   make(map[string]int64),
 		BigDataDigests: make(map[string]digest.Digest),
 		Created:        time.Now().UTC(),
-		Flags:          copyStringInterfaceMap(options.Flags),
+		Flags:          newMapFrom(options.Flags),
 		UIDMap:         copySlicePreferringNil(options.UIDMap),
 		GIDMap:         copySlicePreferringNil(options.GIDMap),
 		volatileStore:  options.Volatile,

diff --git a/images.go b/images.go
@@ -2,7 +2,6 @@ package storage
 
 import (
 	"fmt"
-	"maps"
 	"os"
 	"path/filepath"
 	"slices"
@@ -190,11 +189,11 @@ func copyImage(i *Image) *Image {
 		MappedTopLayers: copySlicePreferringNil(i.MappedTopLayers),
 		Metadata:        i.Metadata,
 		BigDataNames:    copySlicePreferringNil(i.BigDataNames),
-		BigDataSizes:    maps.Clone(i.BigDataSizes),
-		BigDataDigests:  maps.Clone(i.BigDataDigests),
+		BigDataSizes:    copyMapPreferringNil(i.BigDataSizes),
+		BigDataDigests:  copyMapPreferringNil(i.BigDataDigests),
 		Created:         i.Created,
 		ReadOnly:        i.ReadOnly,
-		Flags:           maps.Clone(i.Flags),
+		Flags:           copyMapPreferringNil(i.Flags),
 	}
 }
 
@@ -725,7 +724,7 @@ func (r *imageStore) create(id string, names []string, layer string, options Ima
 		BigDataSizes:   make(map[string]int64),
 		BigDataDigests: make(map[string]digest.Digest),
 		Created:        options.CreationDate,
-		Flags:          copyStringInterfaceMap(options.Flags),
+		Flags:          newMapFrom(options.Flags),
 	}
 	if image.Created.IsZero() {
 		image.Created = time.Now().UTC()

diff --git a/layers.go b/layers.go
@@ -452,7 +452,7 @@ func copyLayer(l *Layer) *Layer {
 		ReadOnly:           l.ReadOnly,
 		volatileStore:      l.volatileStore,
 		BigDataNames:       copySlicePreferringNil(l.BigDataNames),
-		Flags:              maps.Clone(l.Flags),
+		Flags:              copyMapPreferringNil(l.Flags),
 		UIDMap:             copySlicePreferringNil(l.UIDMap),
 		GIDMap:             copySlicePreferringNil(l.GIDMap),
 		UIDs:               copySlicePreferringNil(l.UIDs),
@@ -1403,7 +1403,7 @@ func (r *layerStore) create(id string, parentLayer *Layer, names []string, mount
 		CompressionType:    templateCompressionType,
 		UIDs:               templateUIDs,
 		GIDs:               templateGIDs,
-		Flags:              copyStringInterfaceMap(moreOptions.Flags),
+		Flags:              newMapFrom(moreOptions.Flags),
 		UIDMap:             copySlicePreferringNil(moreOptions.UIDMap),
 		GIDMap:             copySlicePreferringNil(moreOptions.GIDMap),
 		BigDataNames:       []string{},

diff --git a/store.go b/store.go
@@ -1446,7 +1446,7 @@ func (s *store) putLayer(rlstore rwLayerStore, rlstores []roLayerStore, id, pare
 	if lOptions != nil {
 		options = *lOptions
 		options.BigData = slices.Clone(lOptions.BigData)
-		options.Flags = maps.Clone(lOptions.Flags)
+		options.Flags = copyMapPreferringNil(lOptions.Flags)
 	}
 	if options.HostUIDMapping {
 		options.UIDMap = nil
@@ -1762,9 +1762,9 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat
 		options.IDMappingOptions.UIDMap = copySlicePreferringNil(cOptions.IDMappingOptions.UIDMap)
 		options.IDMappingOptions.GIDMap = copySlicePreferringNil(cOptions.IDMappingOptions.GIDMap)
 		options.LabelOpts = copySlicePreferringNil(cOptions.LabelOpts)
-		options.Flags = maps.Clone(cOptions.Flags)
+		options.Flags = copyMapPreferringNil(cOptions.Flags)
 		options.MountOpts = copySlicePreferringNil(cOptions.MountOpts)
-		options.StorageOpt = copyStringStringMap(cOptions.StorageOpt)
+		options.StorageOpt = newMapFrom(cOptions.StorageOpt)
 		options.BigData = slices.Clone(cOptions.BigData)
 	}
 	if options.HostUIDMapping {
@@ -2429,7 +2429,7 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e
 				Digests:      copySlicePreferringNil(i.Digests),
 				Metadata:     i.Metadata,
 				NamesHistory: copySlicePreferringNil(i.NamesHistory),
-				Flags:        copyStringInterfaceMap(i.Flags),
+				Flags:        newMapFrom(i.Flags),
 			}
 			for _, key := range i.BigDataNames {
 				data, err := store.BigData(id, key)
@@ -3664,18 +3664,22 @@ func copySlicePreferringNil[S ~[]E, E any](s S) S {
 	return slices.Clone(s)
 }
 
-func copyStringStringMap(m map[string]string) map[string]string {
-	ret := make(map[string]string, len(m))
-	for k, v := range m {
-		ret[k] = v
+// copyMapPreferringNil returns a shallow clone of map m.
+// If m is empty, a nil is returned.
+//
+// (As of, e.g., Go 1.23, maps.Clone preserves nil, but that’s not a documented promise;
+// and this function turns even non-nil empty maps into nil.)
+func copyMapPreferringNil[K comparable, V any](m map[K]V) map[K]V {
+	if len(m) == 0 {
+		return nil
 	}
-	return ret
+	return maps.Clone(m)
 }
 
-// copyStringInterfaceMap still forces us to assume that the interface{} is
-// a non-pointer scalar value
-func copyStringInterfaceMap(m map[string]interface{}) map[string]interface{} {
-	ret := make(map[string]interface{}, len(m))
+// newMapFrom returns a shallow clone of map m.
+// If m is empty, an empty map is allocated and returned.
+func newMapFrom[K comparable, V any](m map[K]V) map[K]V {
+	ret := make(map[K]V, len(m))
 	for k, v := range m {
 		ret[k] = v
 	}