Throttle stderr progress reporting and other improvements #203
+290
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
korran
reviewed
Feb 4, 2026
korran
reviewed
Feb 4, 2026
ckungler
approved these changes
Feb 9, 2026
Update `libhoth_progress_stderr_func` to only output to stderr when significant progress (at least 1%) has been made, or when at the start/end of an operation. Previously, progress updates could occur as frequently as every byte, leading to excessive syscalls and stderr noise. This change improves performance by ensuring updates are sparse while still providing useful feedback to the user. Changes: - Add `last_reported_numerator` to track reporting state. - Implement a 1% threshold for reporting updates. - Ensure 0% and 100% are always reported.
Changes: - Cache `isatty` check: Move `isatty(STDERR_FILENO)` from the update function to `init`. This avoids making a system call on every progress update, reducing overhead in the hot path. - Rename parameters: Change `numerator`/`denominator` to `current`/`total` to better reflect their semantic meaning in a progress context. - Fix time math: Adjust `ts_subtract` logic to prevent `tv_nsec` from being outside the standard range. Previously, equal nanoseconds triggered an unnecessary borrow, resulting in an invalid `tv_nsec` value of 10^9. - Standardize units: Switch from kB (implied 1000) to KiB (1024) to match standard binary prefixes used in system tools. Usage of `PRIu64` ensures portability across platforms. - Reduce visual jitter: Display remaining time as integer seconds rather than fractional to make the output more stable and readable. - Fix visual artifacts: Append the ANSI clear-line sequence `\033[K` to the output. This ensures that if a progress line is shorter than the previous one, no stale characters remain on the screen.
Add tests for the progress reporting functionality: - Initialization and basic output format. - Throttling of progress updates (only printing on >1% change or total completion). - Throughput calculation and estimated time remaining. - Edge cases such as zero total, zero start time, and non-TTY environments. - Handling of large transfer sizes (multi-GiB). ``` bazel test //protocol:progress_test ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commit messages.