Skip to content

refactor: deduplicate the four copies of the byte-size formatter #1626

Closed

Description

@agatha197

Summary

The GiB/MiB byte-size formatter is duplicated four times: a character-identical pair across src/execution/, and a second near-identical pair inside a single file in src/distributed/.

Background

Pair 1, byte-for-byte identical (same GIB/MIB consts, same "{:.2} GiB ({bytes} bytes)" format strings):

  • src/execution/memory_estimate.rs:1395 — pub fn format_bytes
  • src/execution/quant_advisor.rs:291 — private fn format_bytes

Pair 2, duplicated within one file, differing only by a space before the unit and a < 1024 branch:

  • src/distributed/kv_cache_transfer/benchmark.rs:109 — KvCacheDimensions::total_size_str
  • src/distributed/kv_cache_transfer/benchmark.rs:341 — fn format_bytes

All four locations confirmed present in the source.

Proposed Solution

Delete the quant_advisor copy and import crate::execution::memory_estimate::format_bytes instead. In benchmark.rs, make total_size_str delegate to the file-local format_bytes, picking one spacing convention.

Implementation Notes

  • Existing tests pin the behavior: src/execution/quant_advisor.rs:560,568,574 and the format_bytes_ranges case in benchmark_tests.rs:154.
  • If a spacing convention change is unavoidable when unifying benchmark.rs, make the pick deliberate and note it.

Acceptance Criteria

  • One format_bytes remains in src/execution/; quant_advisor.rs imports it.
  • total_size_str delegates to the file-local format_bytes in benchmark.rs.
  • Existing formatter tests pass unchanged, or with a deliberate, noted spacing pick.

Original Suggestion

Title: refactor: deduplicate the four copies of the byte-size formatter

The GiB/MiB byte-size formatter is duplicated four times: a character-identical pair across src/execution/, and a second near-identical pair inside a single file in src/distributed/.

Evidence

Pair 1 — byte-for-byte identical (same GIB/MIB consts, same "{:.2} GiB ({bytes} bytes)" format strings):

  • src/execution/memory_estimate.rs:1392-1404 — pub fn format_bytes
  • src/execution/quant_advisor.rs:290-301 — private fn format_bytes

Pair 2 — duplicated within one file, differing only by a space before the unit and a < 1024 branch:

  • src/distributed/kv_cache_transfer/benchmark.rs:108-118 — KvCacheDimensions::total_size_str
  • src/distributed/kv_cache_transfer/benchmark.rs:341-350 — fn format_bytes

Suggested fix

Delete the quant_advisor copy and import crate::execution::memory_estimate::format_bytes instead. In benchmark.rs, make total_size_str delegate to format_bytes, picking one spacing convention.

Existing tests already pin the behavior: src/execution/quant_advisor.rs:560,568,574 and benchmark_tests.rs:154 (format_bytes_ranges).

Acceptance criteria

  • One format_bytes remains in src/execution/; quant_advisor.rs imports it
  • total_size_str delegates to the file-local format_bytes in benchmark.rs
  • Existing formatter tests pass unchanged (or with a deliberate, noted spacing pick)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions