From 2edebad6a91eed27fac639d0b51f3e8c5f1aba3c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Wed, 17 Apr 2024 18:32:16 +0800 Subject: [PATCH] chore(ux): optimize progress bar (#1349) Signed-off-by: Billy Zha --- .../display/status/progress/manager.go | 3 +- .../internal/display/status/progress/speed.go | 61 +++++++++++++++++++ .../display/status/progress/speed_test.go | 49 +++++++++++++++ .../display/status/progress/status.go | 42 ++++++------- .../display/status/progress/status_test.go | 9 +++ .../internal/display/status/track/reader.go | 4 +- .../internal/display/status/track/target.go | 2 +- 7 files changed, 141 insertions(+), 29 deletions(-) create mode 100644 cmd/oras/internal/display/status/progress/speed.go create mode 100644 cmd/oras/internal/display/status/progress/speed_test.go diff --git a/cmd/oras/internal/display/status/progress/manager.go b/cmd/oras/internal/display/status/progress/manager.go index 470679976..b17d61e00 100644 --- a/cmd/oras/internal/display/status/progress/manager.go +++ b/cmd/oras/internal/display/status/progress/manager.go @@ -27,7 +27,8 @@ import ( const ( // BufferSize is the size of the status channel buffer. BufferSize = 1 - bufFlushDuration = 200 * time.Millisecond + framePerSecond = 5 + bufFlushDuration = time.Second / framePerSecond ) var errManagerStopped = errors.New("progress output manager has already been stopped") diff --git a/cmd/oras/internal/display/status/progress/speed.go b/cmd/oras/internal/display/status/progress/speed.go new file mode 100644 index 000000000..75a03184d --- /dev/null +++ b/cmd/oras/internal/display/status/progress/speed.go @@ -0,0 +1,61 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package progress + +import "time" + +type speedPoint struct { + time time.Time + offset int64 +} + +type speedWindow struct { + point []speedPoint + next int + size int +} + +// newSpeedWindow creates a new speed window with a given capacity. +func newSpeedWindow(capacity int) *speedWindow { + return &speedWindow{ + point: make([]speedPoint, capacity), + } +} + +// Add adds a done workload to the window. +func (w *speedWindow) Add(time time.Time, offset int64) { + if w.size != len(w.point) { + w.size++ + } + w.point[w.next] = speedPoint{ + time: time, + offset: offset, + } + w.next = (w.next + 1) % len(w.point) +} + +// Mean returns the mean speed of the window with unit of byte per second. +func (w *speedWindow) Mean() float64 { + if w.size < 2 { + // no speed diplayed for first read + return 0 + } + + begin := (w.next - w.size + len(w.point)) % len(w.point) + end := (begin - 1 + w.size) % w.size + + return float64(w.point[end].offset-w.point[begin].offset) / w.point[end].time.Sub(w.point[begin].time).Seconds() +} diff --git a/cmd/oras/internal/display/status/progress/speed_test.go b/cmd/oras/internal/display/status/progress/speed_test.go new file mode 100644 index 000000000..a04063494 --- /dev/null +++ b/cmd/oras/internal/display/status/progress/speed_test.go @@ -0,0 +1,49 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package progress + +import ( + "testing" + "time" +) + +func Test_speedWindow(t *testing.T) { + w := newSpeedWindow(3) + if s := w.Mean(); s != 0 { + t.Errorf("expected 0, got %f", s) + } + + now := time.Now() + w.Add(now, 100) + if s := w.Mean(); s != 0 { + t.Errorf("expected 0, got %f", s) + } + + w.Add(now.Add(1*time.Second), 200) + if s := w.Mean(); s != 100 { + t.Errorf("expected 100, got %f", s) + } + + w.Add(now.Add(4*time.Second), 900) + if s := w.Mean(); s != 200 { + t.Errorf("expected 200, got %f", s) + } + + w.Add(now.Add(5*time.Second), 1400) + if s := w.Mean(); s != 300 { + t.Errorf("expected 300, got %f", s) + } +} diff --git a/cmd/oras/internal/display/status/progress/status.go b/cmd/oras/internal/display/status/progress/status.go index fb7803202..b2abb6ee4 100644 --- a/cmd/oras/internal/display/status/progress/status.go +++ b/cmd/oras/internal/display/status/progress/status.go @@ -43,13 +43,12 @@ var ( // status is used as message to update progress view. type status struct { - done bool // done is true when the end time is set - prompt string - descriptor ocispec.Descriptor - offset int64 - total humanize.Bytes - lastOffset int64 - lastRenderTime time.Time + done bool // done is true when the end time is set + prompt string + descriptor ocispec.Descriptor + offset int64 + total humanize.Bytes + speedWindow *speedWindow startTime time.Time endTime time.Time @@ -60,14 +59,14 @@ type status struct { // newStatus generates a base empty status. func newStatus() *status { return &status{ - offset: -1, - total: humanize.ToBytes(0), - lastRenderTime: time.Now(), + offset: -1, + total: humanize.ToBytes(0), + speedWindow: newSpeedWindow(framePerSecond), } } -// NewStatus generates a status. -func NewStatus(prompt string, descriptor ocispec.Descriptor, offset int64) *status { +// NewStatusMessage generates a status for messaging. +func NewStatusMessage(prompt string, descriptor ocispec.Descriptor, offset int64) *status { return &status{ prompt: prompt, descriptor: descriptor, @@ -159,20 +158,12 @@ func (s *status) String(width int) (string, string) { // calculateSpeed calculates the speed of the progress and update last status. // caller must hold the lock. func (s *status) calculateSpeed() humanize.Bytes { - now := time.Now() - if s.lastRenderTime.IsZero() { - s.lastRenderTime = s.startTime + if s.offset < 0 { + // not started + return humanize.ToBytes(0) } - secondsTaken := now.Sub(s.lastRenderTime).Seconds() - if secondsTaken == 0 { - secondsTaken = bufFlushDuration.Seconds() - } - bytes := float64(s.offset - s.lastOffset) - - s.lastOffset = s.offset - s.lastRenderTime = now - - return humanize.ToBytes(int64(bytes / secondsTaken)) + s.speedWindow.Add(time.Now(), s.offset) + return humanize.ToBytes(int64(s.speedWindow.Mean())) } // durationString returns a viewable TTY string of the status with duration. @@ -216,6 +207,7 @@ func (s *status) Update(n *status) { } if !n.startTime.IsZero() { s.startTime = n.startTime + s.speedWindow.Add(s.startTime, 0) } if !n.endTime.IsZero() { s.endTime = n.endTime diff --git a/cmd/oras/internal/display/status/progress/status_test.go b/cmd/oras/internal/display/status/progress/status_test.go index fab24e73f..417ffb45e 100644 --- a/cmd/oras/internal/display/status/progress/status_test.go +++ b/cmd/oras/internal/display/status/progress/status_test.go @@ -140,3 +140,12 @@ func Test_status_durationString(t *testing.T) { t.Errorf("status.durationString() = %v, want %v", d, want) } } + +func Test_status_calculateSpeed_negative(t *testing.T) { + s := &status{ + offset: -1, + } + if s.calculateSpeed() != humanize.ToBytes(0) { + t.Errorf("status.calculateSpeed() = %v, want 0", s.calculateSpeed()) + } +} diff --git a/cmd/oras/internal/display/status/track/reader.go b/cmd/oras/internal/display/status/track/reader.go index c0301f928..28f647d36 100644 --- a/cmd/oras/internal/display/status/track/reader.go +++ b/cmd/oras/internal/display/status/track/reader.go @@ -66,7 +66,7 @@ func (r *reader) StopManager() { // Done sends message to mark the tracked progress as complete. func (r *reader) Done() { - r.status <- progress.NewStatus(r.donePrompt, r.descriptor, r.descriptor.Size) + r.status <- progress.NewStatusMessage(r.donePrompt, r.descriptor, r.descriptor.Size) r.status <- progress.EndTiming() } @@ -95,7 +95,7 @@ func (r *reader) Read(p []byte) (int, error) { } for { select { - case r.status <- progress.NewStatus(r.actionPrompt, r.descriptor, r.offset): + case r.status <- progress.NewStatusMessage(r.actionPrompt, r.descriptor, r.offset): // purge the channel until successfully pushed return n, err case <-r.status: diff --git a/cmd/oras/internal/display/status/track/target.go b/cmd/oras/internal/display/status/track/target.go index c57062702..9bd54d240 100644 --- a/cmd/oras/internal/display/status/track/target.go +++ b/cmd/oras/internal/display/status/track/target.go @@ -115,7 +115,7 @@ func (t *graphTarget) Prompt(desc ocispec.Descriptor, prompt string) error { return err } defer close(status) - status <- progress.NewStatus(prompt, desc, desc.Size) + status <- progress.NewStatusMessage(prompt, desc, desc.Size) status <- progress.EndTiming() return nil }