From d37de8316ee51cd74b5881271142cf0803241c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20L=C3=B3pez?= <1953782+julio-lopez@users.noreply.github.com> Date: Mon, 26 Aug 2024 17:26:32 -0700 Subject: [PATCH] refactor(general): generalize `units` package (#4075) 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. --- cli/command_benchmark_compression.go | 10 ++--- cli/command_benchmark_crypto.go | 2 +- cli/command_benchmark_ecc.go | 6 +-- cli/command_benchmark_encryption.go | 2 +- cli/command_benchmark_hashing.go | 2 +- cli/command_benchmark_splitters.go | 8 ++-- cli/command_blob_stats.go | 2 +- cli/command_content_stats.go | 2 +- cli/command_policy_show.go | 2 +- cli/command_repository_set_parameters.go | 47 +++++++++--------------- cli/command_repository_status.go | 6 +-- cli/show_utils.go | 9 +++-- internal/units/units.go | 28 +++++++++----- tests/tools/kopiaclient/kopiaclient.go | 2 +- 14 files changed, 64 insertions(+), 64 deletions(-) diff --git a/cli/command_benchmark_compression.go b/cli/command_benchmark_compression.go index e6e05c11490..5df1d26d96f 100644 --- a/cli/command_benchmark_compression.go +++ b/cli/command_benchmark_compression.go @@ -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": @@ -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) @@ -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() @@ -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, diff --git a/cli/command_benchmark_crypto.go b/cli/command_benchmark_crypto.go index 97ca475abe3..bf04337e5ad 100644 --- a/cli/command_benchmark_crypto.go +++ b/cli/command_benchmark_crypto.go @@ -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) diff --git a/cli/command_benchmark_ecc.go b/cli/command_benchmark_ecc.go index ee5aeac101c..8e0458a15c1 100644 --- a/cli/command_benchmark_ecc.go +++ b/cli/command_benchmark_ecc.go @@ -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 { diff --git a/cli/command_benchmark_encryption.go b/cli/command_benchmark_encryption.go index 31cbe053bfb..eceb4869030 100644 --- a/cli/command_benchmark_encryption.go +++ b/cli/command_benchmark_encryption.go @@ -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) diff --git a/cli/command_benchmark_hashing.go b/cli/command_benchmark_hashing.go index 555f2b8215f..5374f1a1bbc 100644 --- a/cli/command_benchmark_hashing.go +++ b/cli/command_benchmark_hashing.go @@ -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) diff --git a/cli/command_benchmark_splitters.go b/cli/command_benchmark_splitters.go index 13a76f47313..f2fdb0df4e4 100644 --- a/cli/command_benchmark_splitters.go +++ b/cli/command_benchmark_splitters.go @@ -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, ) @@ -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) diff --git a/cli/command_blob_stats.go b/cli/command_blob_stats.go index bcce121895e..1aa38fd4d8e 100644 --- a/cli/command_blob_stats.go +++ b/cli/command_blob_stats.go @@ -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) diff --git a/cli/command_content_stats.go b/cli/command_content_stats.go index 981b2eb0a8a..b93b97ac1e3 100644 --- a/cli/command_content_stats.go +++ b/cli/command_content_stats.go @@ -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) diff --git a/cli/command_policy_show.go b/cli/command_policy_show.go index 3ceb9b5283a..1e314a0ea85 100644 --- a/cli/command_policy_show.go +++ b/cli/command_policy_show.go @@ -488,5 +488,5 @@ func valueOrNotSetOptionalInt64Bytes(p *policy.OptionalInt64) string { return "-" } - return units.BytesString(int64(*p)) + return units.BytesString(*p) } diff --git a/cli/command_repository_set_parameters.go b/cli/command_repository_set_parameters.go index e47491750df..71131c076a2 100644 --- a/cli/command_repository_set_parameters.go +++ b/cli/command_repository_set_parameters.go @@ -67,7 +67,7 @@ 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 } @@ -75,21 +75,10 @@ func (c *commandRepositorySetParameters) setSizeMBParameter(ctx context.Context, *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 } @@ -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 } @@ -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 } @@ -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 = "" @@ -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") } @@ -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) diff --git a/cli/command_repository_status.go b/cli/command_repository_status.go index 7121c819169..7eb2cf22212 100644 --- a/cli/command_repository_status.go +++ b/cli/command_repository_status.go @@ -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: @@ -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) diff --git a/cli/show_utils.go b/cli/show_utils.go index f903190f4b4..1099ebb180d 100644 --- a/cli/show_utils.go +++ b/cli/show_utils.go @@ -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" @@ -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 { diff --git a/internal/units/units.go b/internal/units/units.go index c3d28cc67c5..6b0b5089a1d 100644 --- a/internal/units/units.go +++ b/internal/units/units.go @@ -6,6 +6,8 @@ import ( "os" "strconv" "strings" + + "golang.org/x/exp/constraints" ) //nolint:gochecknoglobals @@ -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) @@ -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) } @@ -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, "") } diff --git a/tests/tools/kopiaclient/kopiaclient.go b/tests/tools/kopiaclient/kopiaclient.go index 0b56952d01f..d1bc6b90217 100644 --- a/tests/tools/kopiaclient/kopiaclient.go +++ b/tests/tools/kopiaclient/kopiaclient.go @@ -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