Skip to content

Conversation

@karanabe
Copy link
Contributor

Fixed an issue where the locale was not initialised in threads created by the top command. Call ensure_uptime_locale() from all public uptime so the feature self-initialises its locale.

The first render in procps/top is still untranslated because uucore::bin! initialises only the “top” locale on the main thread. Addressing this would require broader changes—specifically updating locale.rs to support multiple concurrent locales used by different modules—so it is left out of this PR.

Fixes #8962

Store a thread-local flag so ensure_uptime_locale() only runs
setup_localization("uptime") once per thread, and call it from every
public API entry point. This lets library consumers get translated
strings even when the current thread did not explicitly initialize the
locale.
@karanabe karanabe force-pushed the fix/uptime-dependency branch from 463208e to 2fa7a32 Compare October 26, 2025 10:43
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 26, 2025

CodSpeed Performance Report

Merging this PR will degrade performance by 4.08%

Comparing karanabe:fix/uptime-dependency (15ed57b) with main (a1545ef)

Summary

⚡ 2 improved benchmarks
❌ 1 regressed benchmark
✅ 279 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory du_wide_tree[(5000, 500)] 1.2 MB 1.2 MB +3.49%
Memory cp_large_file[16] 115.3 KB 120.2 KB -4.08%
Memory dd_copy_partial 133.5 KB 129.1 KB +3.37%

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)

@anastygnome
Copy link
Contributor

Oh, the fail is due to the test running in french!

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cksum/cksum-c is no longer failing!
Congrats! The gnu test tests/cksum/cksum-sha3 is no longer failing!

@karanabe
Copy link
Contributor Author

@anastygnome Thank you for letting me know!

@karanabe karanabe requested a review from sylvestre November 2, 2025 02:40
@anastygnome
Copy link
Contributor

@sylvestre Hey, I believe the perf regression is normal here, is this okay to merge?

Store a thread-local flag so ensure_uptime_locale() only runs
setup_localization("uptime") once per thread, and call it from every
public API entry point. This lets library consumers get translated
strings even when the current thread did not explicitly initialize the
locale.
@sylvestre sylvestre force-pushed the fix/uptime-dependency branch from f549910 to dca827e Compare November 9, 2025 13:22
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

/// e.g. "load average: 0.00, 0.00, 0.00"
#[inline]
pub fn get_formatted_loadavg() -> UResult<String> {
ensure_uptime_locale();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not a fan to duplicate this call in every functions of uptime
can we do it only at load time in one place ? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. Unfortunately setup_localization uses a thread‑local LOCALIZER (see src/uucore/src/lib/mods/locale.rs), so initializing once at load time only affects that thread. If uptime is called from another thread, translate! returns the message id instead of a localized string. ensure_uptime_locale() is guarded by a thread‑local flag, so after the first call per thread it’s effectively a no‑op and avoids the “already initialized” error. This makes the API safe for multi‑threaded consumers (e.g. a refresh thread), while keeping the runtime cost minimal.

@sylvestre
Copy link
Contributor

sorry, it needs to be rebased!

@karanabe
Copy link
Contributor Author

Thanks for the review! I’ll rebase and take care of the review comments.

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.

l10n: uucore/uptime has dependency on uptime

3 participants