-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
uptime: locale init per thread #9028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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.
463208e to
2fa7a32
Compare
CodSpeed Performance ReportMerging this PR will degrade performance by 4.08%Comparing Summary
Performance Changes
Footnotes
|
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
Oh, the fail is due to the test running in french! |
|
GNU testsuite comparison: |
|
@anastygnome Thank you for letting me know! |
|
@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.
f549910 to
dca827e
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
| /// e.g. "load average: 0.00, 0.00, 0.00" | ||
| #[inline] | ||
| pub fn get_formatted_loadavg() -> UResult<String> { | ||
| ensure_uptime_locale(); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
sorry, it needs to be rebased! |
|
Thanks for the review! I’ll rebase and take care of the review comments. |
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 updatinglocale.rsto support multiple concurrent locales used by different modules—so it is left out of this PR.Fixes #8962