Skip to content

UPSTREAM PR #18276: tools : use common_log_pause to fix fit-params output race#669

Open
loci-dev wants to merge 1 commit intomainfrom
upstream-PR18276-branch_Aadeshveer-fix-fit-params-race
Open

UPSTREAM PR #18276: tools : use common_log_pause to fix fit-params output race#669
loci-dev wants to merge 1 commit intomainfrom
upstream-PR18276-branch_Aadeshveer-fix-fit-params-race

Conversation

@loci-dev
Copy link
Copy Markdown

Mirrored from ggml-org/llama.cpp#18276

Replaced the std::this_thread::sleep_for(10ms) hack with common_log_pause(common_log_main()) in llama-fit-params.

Reasoning:
The previous 10ms delay was intended to let the async logger drain its queue to stderr before the main thread printed the result to stdout. However, on high-performance or loaded systems, 10ms is not guaranteed to be enough, leading to interleaved output (see Issue #18085).

common_log_pause explicitly waits for the background worker to drain the log queue, ensuring deterministic separation between logs and the final result.

Fixes #18085

@loci-review
Copy link
Copy Markdown

loci-review bot commented Dec 23, 2025

Explore the complete analysis inside the Version Insights

Performance Analysis Summary - PR #669

Analysis Overview

PR #669 modifies a single line in tools/fit-params/fit-params.cpp, replacing a 10ms sleep with common_log_flush(common_log_main()) to fix output race conditions. Performance analysis shows no measurable impact across all analyzed binaries and functions.

Performance Metrics:

  • Response Time change: 0% across all functions
  • Throughput Time change: 0% across all functions
  • Power consumption change: -0.0001% system-wide (maximum -1 nJ in llama-tts)

Code Modification Scope:

  • 1 file changed
  • 1 line modified
  • 0 performance-critical functions affected
  • Change isolated to utility tool, not inference path

The modification addresses correctness without affecting computational performance. All core libraries and inference functions remain unchanged.

@loci-dev loci-dev force-pushed the main branch 27 times, most recently from f7cdf84 to 701f648 Compare December 26, 2025 23:08
@loci-dev loci-dev force-pushed the main branch 30 times, most recently from 027726b to 226759a Compare January 1, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants