Skip to content

Commit

Permalink
refactor(general): generalize units package (kopia#4075)
Browse files Browse the repository at this point in the history 
Generalize a couple of functions in the units package using generics.
This allows removing duplicate code and simplifying callers by removing unnecessary integer conversions.

Additional cleanups:

- make "/s" part of the Printf format string ;
- simplify setSizeMBParameter;
- generalize cli.maybeHumanReadable*` helpers;
- remove unneeded receiver in commandRepositorySetParameters helpers.
  • Loading branch information
@julio-lopez
julio-lopez authored Aug 27, 2024
1 parent 9972c21 commit d37de83
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 64 deletions.
10 changes: 5 additions & 5 deletions cli/command_benchmark_compression.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (c *commandBenchmarkCompression) run(ctx context.Context) error {
}
}

log(ctx).Infof("Will repeat each benchmark %v times per compression method (total %v). Override with --repeat=N.", repeatCount, units.BytesString(int64(repeatCount*len(data))))
log(ctx).Infof("Will repeat each benchmark %v times per compression method (total %v). Override with --repeat=N.", repeatCount, units.BytesString(repeatCount*len(data)))

switch c.operations {
case "compress":
Expand Down Expand Up @@ -168,7 +168,7 @@ func (c *commandBenchmarkCompression) run(ctx context.Context) error {
func (c *commandBenchmarkCompression) runCompression(ctx context.Context, data []byte, repeatCount int, algorithms map[compression.Name]compression.Compressor) error {
var results []compressionBechmarkResult

log(ctx).Infof("Compressing input file %q (%v) using %v compression methods.", c.dataFile, units.BytesString(int64(len(data))), len(algorithms))
log(ctx).Infof("Compressing input file %q (%v) using %v compression methods.", c.dataFile, units.BytesString(len(data)), len(algorithms))

for name, comp := range algorithms {
log(ctx).Infof("Benchmarking compressor '%v'...", name)
Expand Down Expand Up @@ -243,7 +243,7 @@ func (c *commandBenchmarkCompression) runCompression(ctx context.Context, data [
func (c *commandBenchmarkCompression) runDecompression(ctx context.Context, data []byte, repeatCount int, algorithms map[compression.Name]compression.Compressor) error {
var results []compressionBechmarkResult

log(ctx).Infof("Decompressing input file %q (%v) using %v compression methods.", c.dataFile, units.BytesString(int64(len(data))), len(algorithms))
log(ctx).Infof("Decompressing input file %q (%v) using %v compression methods.", c.dataFile, units.BytesString(len(data)), len(algorithms))

var compressedInput gather.WriteBuffer
defer compressedInput.Close()
Expand Down Expand Up @@ -335,11 +335,11 @@ func (c *commandBenchmarkCompression) printResults(results []compressionBechmark
maybeDeprecated = " (deprecated)"
}

c.out.printStdout("%3d. %-26v %-12v %-12v %-8v %v%v",
c.out.printStdout("%3d. %-26v %-12v %-12v/s %-8v %v%v",
ndx,
r.compression,
units.BytesString(r.compressedSize),
units.BytesString(int64(r.throughput))+"/s",
units.BytesString(r.throughput),
r.allocations,
units.BytesString(r.allocBytes),
maybeDeprecated,
Expand Down
2 changes: 1 addition & 1 deletion cli/command_benchmark_crypto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (c *commandBenchmarkCrypto) run(ctx context.Context) error {
c.out.printStdout("-----------------------------------------------------------------\n")

for ndx, r := range results {
c.out.printStdout("%3d. %-20v %-30v %v / second", ndx, r.hash, r.encryption, units.BytesString(int64(r.throughput)))
c.out.printStdout("%3d. %-20v %-30v %v / second", ndx, r.hash, r.encryption, units.BytesString(r.throughput))

if c.optionPrint {
c.out.printStdout(", --block-hash=%s --encryption=%s", r.hash, r.encryption)
Expand Down
6 changes: 3 additions & 3 deletions cli/command_benchmark_ecc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func (c *commandBenchmarkEcc) run(ctx context.Context) error {

for ndx, r := range results {
c.out.printStdout("%3d. %-30v %12v/s %12v/s %6v%% [%v]", ndx, r.ecc,
units.BytesString(int64(r.throughputEncoding)),
units.BytesString(int64(r.throughputDecoding)),
units.BytesString(r.throughputEncoding),
units.BytesString(r.throughputDecoding),
int(math.Round(r.growth*100)), //nolint:mnd
units.BytesString(int64(r.size)),
units.BytesString(r.size),
)

if c.optionPrint {
Expand Down
2 changes: 1 addition & 1 deletion cli/command_benchmark_encryption.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (c *commandBenchmarkEncryption) run(ctx context.Context) error {
c.out.printStdout("-----------------------------------------------------------------\n")

for ndx, r := range results {
c.out.printStdout("%3d. %-30v %v / second", ndx, r.encryption, units.BytesString(int64(r.throughput)))
c.out.printStdout("%3d. %-30v %v / second", ndx, r.encryption, units.BytesString(r.throughput))

if c.optionPrint {
c.out.printStdout(", --encryption=%s", r.encryption)
Expand Down
2 changes: 1 addition & 1 deletion cli/command_benchmark_hashing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (c *commandBenchmarkHashing) run(ctx context.Context) error {
c.out.printStdout("-----------------------------------------------------------------\n")

for ndx, r := range results {
c.out.printStdout("%3d. %-20v %v / second", ndx, r.hash, units.BytesString(int64(r.throughput)))
c.out.printStdout("%3d. %-20v %v / second", ndx, r.hash, units.BytesString(r.throughput))

if c.optionPrint {
c.out.printStdout(", --block-hash=%s", r.hash)
Expand Down
8 changes: 4 additions & 4 deletions cli/command_benchmark_splitters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ func (c *commandBenchmarkSplitters) run(ctx context.Context) error { //nolint:fu
int64(bytesPerSecond),
}

c.out.printStdout("%-25v %12v count:%v min:%v 10th:%v 25th:%v 50th:%v 75th:%v 90th:%v max:%v\n",
c.out.printStdout("%-25v %12v/s count:%v min:%v 10th:%v 25th:%v 50th:%v 75th:%v 90th:%v max:%v\n",
r.splitter,
units.BytesString(r.bytesPerSecond)+"/s",
units.BytesString(r.bytesPerSecond),
r.segmentCount,
r.min, r.p10, r.p25, r.p50, r.p75, r.p90, r.max,
)
Expand All @@ -139,10 +139,10 @@ func (c *commandBenchmarkSplitters) run(ctx context.Context) error { //nolint:fu
c.out.printStdout("-----------------------------------------------------------------\n")

for ndx, r := range results {
c.out.printStdout("%3v. %-25v %-12v count:%v min:%v 10th:%v 25th:%v 50th:%v 75th:%v 90th:%v max:%v\n",
c.out.printStdout("%3v. %-25v %-12v/s count:%v min:%v 10th:%v 25th:%v 50th:%v 75th:%v 90th:%v max:%v\n",
ndx,
r.splitter,
units.BytesString(r.bytesPerSecond)+"/s",
units.BytesString(r.bytesPerSecond),
r.segmentCount,
r.min, r.p10, r.p25, r.p50, r.p75, r.p90, r.max)

Expand Down
2 changes: 1 addition & 1 deletion cli/command_blob_stats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (c *commandBlobStats) run(ctx context.Context, rep repo.DirectRepository) e
return errors.Wrap(err, "error listing blobs")
}

sizeToString := units.BytesString
sizeToString := units.BytesString[int64]
if c.raw {
sizeToString = func(l int64) string {
return strconv.FormatInt(l, 10)
Expand Down
2 changes: 1 addition & 1 deletion cli/command_content_stats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (c *commandContentStats) run(ctx context.Context, rep repo.DirectRepository
return errors.Wrap(err, "error calculating totals")
}

sizeToString := units.BytesString
sizeToString := units.BytesString[int64]
if c.raw {
sizeToString = func(l int64) string {
return strconv.FormatInt(l, 10)
Expand Down
2 changes: 1 addition & 1 deletion cli/command_policy_show.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,5 +488,5 @@ func valueOrNotSetOptionalInt64Bytes(p *policy.OptionalInt64) string {
return "-"
}

return units.BytesString(int64(*p))
return units.BytesString(*p)
}
47 changes: 18 additions & 29 deletions cli/command_repository_set_parameters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,29 +67,18 @@ func (c *commandRepositorySetParameters) setup(svc appServices, parent commandPa
c.svc = svc
}

func (c *commandRepositorySetParameters) setSizeMBParameter(ctx context.Context, v int, desc string, dst *int, anyChange *bool) {
func setSizeMBParameter[I ~int | ~int32 | ~int64 | ~uint | ~uint32 | ~uint64](ctx context.Context, v I, desc string, dst *I, anyChange *bool) {
if v == 0 {
return
}

*dst = v << 20 //nolint:mnd
*anyChange = true

log(ctx).Infof(" - setting %v to %v.\n", desc, units.BytesString(int64(v)<<20)) //nolint:mnd
log(ctx).Infof(" - setting %v to %v.\n", desc, units.BytesString(*dst))
}

func (c *commandRepositorySetParameters) setInt64SizeMBParameter(ctx context.Context, v int64, desc string, dst *int64, anyChange *bool) {
if v == 0 {
return
}

*dst = v << 20 //nolint:mnd
*anyChange = true

log(ctx).Infof(" - setting %v to %v.\n", desc, units.BytesString(v<<20)) //nolint:mnd
}

func (c *commandRepositorySetParameters) setIntParameter(ctx context.Context, v int, desc string, dst *int, anyChange *bool) {
func setIntParameter(ctx context.Context, v int, desc string, dst *int, anyChange *bool) {
if v == 0 {
return
}
Expand All @@ -100,7 +89,7 @@ func (c *commandRepositorySetParameters) setIntParameter(ctx context.Context, v
log(ctx).Infof(" - setting %v to %v.\n", desc, v)
}

func (c *commandRepositorySetParameters) setDurationParameter(ctx context.Context, v time.Duration, desc string, dst *time.Duration, anyChange *bool) {
func setDurationParameter(ctx context.Context, v time.Duration, desc string, dst *time.Duration, anyChange *bool) {
if v == 0 {
return
}
Expand All @@ -111,7 +100,7 @@ func (c *commandRepositorySetParameters) setDurationParameter(ctx context.Contex
log(ctx).Infof(" - setting %v to %v.\n", desc, v)
}

func (c *commandRepositorySetParameters) setRetentionModeParameter(ctx context.Context, v blob.RetentionMode, desc string, dst *blob.RetentionMode, anyChange *bool) {
func setRetentionModeParameter(ctx context.Context, v blob.RetentionMode, desc string, dst *blob.RetentionMode, anyChange *bool) {
if !v.IsValid() {
return
}
Expand Down Expand Up @@ -165,7 +154,7 @@ func updateEpochParameters(mp *format.MutableParameters, anyChange, upgradeToEpo
}
}

func (c *commandRepositorySetParameters) disableBlobRetention(ctx context.Context, blobcfg *format.BlobStorageConfiguration, anyChange *bool) {
func disableBlobRetention(ctx context.Context, blobcfg *format.BlobStorageConfiguration, anyChange *bool) {
log(ctx).Info("disabling blob retention")

blobcfg.RetentionMode = ""
Expand Down Expand Up @@ -196,12 +185,12 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc
updateEpochParameters(&mp, &anyChange, &upgradeToEpochManager)
}

c.setSizeMBParameter(ctx, c.maxPackSizeMB, "maximum pack size", &mp.MaxPackSize, &anyChange)
setSizeMBParameter(ctx, c.maxPackSizeMB, "maximum pack size", &mp.MaxPackSize, &anyChange)

// prevent downgrade of index format
if c.indexFormatVersion != 0 && c.indexFormatVersion != mp.IndexVersion {
if c.indexFormatVersion > mp.IndexVersion {
c.setIntParameter(ctx, c.indexFormatVersion, "index format version", &mp.IndexVersion, &anyChange)
setIntParameter(ctx, c.indexFormatVersion, "index format version", &mp.IndexVersion, &anyChange)
} else {
return errors.Errorf("index format version can only be upgraded")
}
Expand All @@ -210,20 +199,20 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc
if c.retentionMode == "none" {
if blobcfg.IsRetentionEnabled() {
// disable blob retention if already enabled
c.disableBlobRetention(ctx, &blobcfg, &anyChange)
disableBlobRetention(ctx, &blobcfg, &anyChange)
}
} else {
c.setRetentionModeParameter(ctx, blob.RetentionMode(c.retentionMode), "storage backend blob retention mode", &blobcfg.RetentionMode, &anyChange)
c.setDurationParameter(ctx, c.retentionPeriod, "storage backend blob retention period", &blobcfg.RetentionPeriod, &anyChange)
setRetentionModeParameter(ctx, blob.RetentionMode(c.retentionMode), "storage backend blob retention mode", &blobcfg.RetentionMode, &anyChange)
setDurationParameter(ctx, c.retentionPeriod, "storage backend blob retention period", &blobcfg.RetentionPeriod, &anyChange)
}

c.setDurationParameter(ctx, c.epochMinDuration, "minimum epoch duration", &mp.EpochParameters.MinEpochDuration, &anyChange)
c.setDurationParameter(ctx, c.epochRefreshFrequency, "epoch refresh frequency", &mp.EpochParameters.EpochRefreshFrequency, &anyChange)
c.setDurationParameter(ctx, c.epochCleanupSafetyMargin, "epoch cleanup safety margin", &mp.EpochParameters.CleanupSafetyMargin, &anyChange)
c.setIntParameter(ctx, c.epochAdvanceOnCount, "epoch advance on count", &mp.EpochParameters.EpochAdvanceOnCountThreshold, &anyChange)
c.setInt64SizeMBParameter(ctx, c.epochAdvanceOnSizeMB, "epoch advance on total size", &mp.EpochParameters.EpochAdvanceOnTotalSizeBytesThreshold, &anyChange)
c.setIntParameter(ctx, c.epochDeleteParallelism, "epoch delete parallelism", &mp.EpochParameters.DeleteParallelism, &anyChange)
c.setIntParameter(ctx, c.epochCheckpointFrequency, "epoch checkpoint frequency", &mp.EpochParameters.FullCheckpointFrequency, &anyChange)
setDurationParameter(ctx, c.epochMinDuration, "minimum epoch duration", &mp.EpochParameters.MinEpochDuration, &anyChange)
setDurationParameter(ctx, c.epochRefreshFrequency, "epoch refresh frequency", &mp.EpochParameters.EpochRefreshFrequency, &anyChange)
setDurationParameter(ctx, c.epochCleanupSafetyMargin, "epoch cleanup safety margin", &mp.EpochParameters.CleanupSafetyMargin, &anyChange)
setIntParameter(ctx, c.epochAdvanceOnCount, "epoch advance on count", &mp.EpochParameters.EpochAdvanceOnCountThreshold, &anyChange)
setSizeMBParameter(ctx, c.epochAdvanceOnSizeMB, "epoch advance on total size", &mp.EpochParameters.EpochAdvanceOnTotalSizeBytesThreshold, &anyChange)
setIntParameter(ctx, c.epochDeleteParallelism, "epoch delete parallelism", &mp.EpochParameters.DeleteParallelism, &anyChange)
setIntParameter(ctx, c.epochCheckpointFrequency, "epoch checkpoint frequency", &mp.EpochParameters.FullCheckpointFrequency, &anyChange)

requiredFeatures = c.addRemoveUpdateRequiredFeatures(requiredFeatures, &anyChange)

Expand Down
6 changes: 3 additions & 3 deletions cli/command_repository_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ func (c *commandRepositoryStatus) run(ctx context.Context, rep repo.Repository)

switch cp, err := dr.BlobVolume().GetCapacity(ctx); {
case err == nil:
c.out.printStdout("Storage capacity: %v\n", units.BytesString(int64(cp.SizeB)))
c.out.printStdout("Storage available: %v\n", units.BytesString(int64(cp.FreeB)))
c.out.printStdout("Storage capacity: %v\n", units.BytesString(cp.SizeB))
c.out.printStdout("Storage available: %v\n", units.BytesString(cp.FreeB))
case errors.Is(err, blob.ErrNotAVolume):
c.out.printStdout("Storage capacity: unbounded\n")
default:
Expand Down Expand Up @@ -190,7 +190,7 @@ func (c *commandRepositoryStatus) run(ctx context.Context, rep repo.Repository)

c.outputRequiredFeatures(ctx, dr)

c.out.printStdout("Max pack length: %v\n", units.BytesString(int64(mp.MaxPackSize)))
c.out.printStdout("Max pack length: %v\n", units.BytesString(mp.MaxPackSize))
c.out.printStdout("Index Format: v%v\n", mp.IndexVersion)

emgr, epochMgrEnabled, emerr := dr.ContentReader().EpochManager(ctx)
Expand Down
9 changes: 5 additions & 4 deletions cli/show_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"time"

"github.com/pkg/errors"
"golang.org/x/exp/constraints"

"github.com/kopia/kopia/internal/iocopy"
"github.com/kopia/kopia/internal/units"
Expand Down Expand Up @@ -53,20 +54,20 @@ func showContentWithFlags(w io.Writer, rd io.Reader, unzip, indentJSON bool) err
return nil
}

func maybeHumanReadableBytes(enable bool, value int64) string {
func maybeHumanReadableBytes[I constraints.Integer](enable bool, value I) string {
if enable {
return units.BytesString(value)
}

return strconv.FormatInt(value, 10)
return strconv.FormatInt(int64(value), 10)
}

func maybeHumanReadableCount(enable bool, value int64) string {
func maybeHumanReadableCount[I constraints.Integer](enable bool, value I) string {
if enable {
return units.Count(value)
}

return strconv.FormatInt(value, 10)
return strconv.FormatInt(int64(value), 10)
}

func formatTimestamp(ts time.Time) string {
Expand Down
28 changes: 19 additions & 9 deletions internal/units/units.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"os"
"strconv"
"strings"

"golang.org/x/exp/constraints"
)

//nolint:gochecknoglobals
Expand All @@ -22,7 +24,15 @@ func niceNumber(f float64) string {
return strings.TrimRight(strings.TrimRight(fmt.Sprintf("%.1f", f), "0"), ".")
}

func toDecimalUnitString(f, thousand float64, prefixes []string, suffix string) string {
type realNumber interface {
constraints.Integer | constraints.Float
}

func toDecimalUnitString[T realNumber](f T, thousand float64, prefixes []string, suffix string) string {
return toDecimalUnitStringImp(float64(f), thousand, prefixes, suffix)
}

func toDecimalUnitStringImp(f, thousand float64, prefixes []string, suffix string) string {
for i := range prefixes {
if f < 0.9*thousand {
return fmt.Sprintf("%v %v%v", niceNumber(f), prefixes[i], suffix)
Expand All @@ -35,19 +45,19 @@ func toDecimalUnitString(f, thousand float64, prefixes []string, suffix string)
}

// BytesStringBase10 formats the given value as bytes with the appropriate base-10 suffix (KB, MB, GB, ...)
func BytesStringBase10(b int64) string {
func BytesStringBase10[T realNumber](b T) string {
//nolint:mnd
return toDecimalUnitString(float64(b), 1000, base10UnitPrefixes, "B")
return toDecimalUnitString(b, 1000, base10UnitPrefixes, "B")
}

// BytesStringBase2 formats the given value as bytes with the appropriate base-2 suffix (KiB, MiB, GiB, ...)
func BytesStringBase2(b int64) string {
func BytesStringBase2[T realNumber](b T) string {
//nolint:mnd
return toDecimalUnitString(float64(b), 1024.0, base2UnitPrefixes, "B")
return toDecimalUnitString(b, 1024.0, base2UnitPrefixes, "B")
}

// BytesString formats the given value as bytes with the unit provided from the environment.
func BytesString(b int64) string {
func BytesString[T realNumber](b T) string {
if v, _ := strconv.ParseBool(os.Getenv(bytesStringBase2Envar)); v {
return BytesStringBase2(b)
}
Expand All @@ -56,13 +66,13 @@ func BytesString(b int64) string {
}

// BytesPerSecondsString formats the given value bytes per second with the appropriate base-10 suffix (KB/s, MB/s, GB/s, ...)
func BytesPerSecondsString(bps float64) string {
func BytesPerSecondsString[T realNumber](bps T) string {
//nolint:mnd
return toDecimalUnitString(bps, 1000, base10UnitPrefixes, "B/s")
}

// Count returns the given number with the appropriate base-10 suffix (K, M, G, ...)
func Count(v int64) string {
func Count[T constraints.Integer](v T) string {
//nolint:mnd
return toDecimalUnitString(float64(v), 1000, base10UnitPrefixes, "")
return toDecimalUnitString(v, 1000, base10UnitPrefixes, "")
}
2 changes: 1 addition & 1 deletion tests/tools/kopiaclient/kopiaclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (kc *KopiaClient) SnapshotRestore(ctx context.Context, key string) ([]byte,
return nil, err
}

log.Printf("restored %v", units.BytesString(int64(len(val))))
log.Printf("restored %v", units.BytesString(len(val)))

if err := r.Close(ctx); err != nil {
return nil, err
Expand Down

0 comments on commit d37de83

Please sign in to comment.