Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove String.format calls #2733

Open
alecgrieser opened this issue May 22, 2024 · 0 comments · May be fixed by #2734
Open

Remove String.format calls #2733

alecgrieser opened this issue May 22, 2024 · 0 comments · May be fixed by #2734
Assignees
@alecgrieser

Comments

@alecgrieser
Copy link
Contributor

We've encountered a few problems with String.format. Namely:

  1. The performance can be pretty bad. We've noted cases where hotspots can be attributed to String.format calls in hot code paths. While we could theoretically remove the call only in hot paths, it may be easier to just remove the call entirely.
  2. Integer formatting can be surprising (see Surprising and inconsistent behavior depending on default locale #2672), as it attempts to use locale specific formatting. This is surprising behavior to the user, as the displayed text is not in a localized string but in the middle of what would otherwise be English

We should consider removing them all and just relying on String concatenation, which is faster and more reliable.
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue May 22, 2024
@alecgrieser
Resolves FoundationDB#2733: Remove String.format calls
6252eea 
This removes as many `String.format` calls as could be done. For the most part, these were just using `%s` or `%d`, which can be replaced with String concatenation. For error messages, an effort was made to move the interpolated data into log message keys where appropriate so that the error messages are static strings (as much as possible), which is useful when trying to group together related errors in a production environment.

This resolves FoundationDB#2733.
@alecgrieser alecgrieser linked a pull request May 22, 2024 that will close this issue
Open
@alecgrieser alecgrieser self-assigned this May 22, 2024
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue May 23, 2024
@alecgrieser
Resolves FoundationDB#2733: Remove String.format calls
1fdcf39 
This removes as many `String.format` calls as could be done. For the most part, these were just using `%s` or `%d`, which can be replaced with String concatenation. For error messages, an effort was made to move the interpolated data into log message keys where appropriate so that the error messages are static strings (as much as possible), which is useful when trying to group together related errors in a production environment.

This resolves FoundationDB#2733.
alecgrieser added a commit to alecgrieser/fdb-record-layer that referenced this issue May 23, 2024
@alecgrieser
Resolves FoundationDB#2733: Remove String.format calls
87c9218 
This removes as many `String.format` calls as could be done. For the most part, these were just using `%s` or `%d`, which can be replaced with String concatenation. For error messages, an effort was made to move the interpolated data into log message keys where appropriate so that the error messages are static strings (as much as possible), which is useful when trying to group together related errors in a production environment.

This resolves FoundationDB#2733.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alecgrieser alecgrieser

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Resolves #2733: Remove String.format calls alecgrieser/fdb-record-layer
1 participant
@alecgrieser