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.

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

Row.c code shrink and adding a new format for Row_printNanoseconds() #1579

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Explorer09
Copy link
Contributor

The first three commits are code shrinks.
The last one commit adds a new format for Row_printNanoseconds() because I feel I missed a chance of allowing one extra digit of precision to be printed.
The last commit is optional. If people feel the new format is not worth it, I can drop that commit.

(Follow-up of #1425)

* Reduce length of the format strings by removing redundant field
  width specifications (e.g. "%1u" -> "%u").
* Merge some identical function calls in conditionals.

Signed-off-by: Kang-Che Sung <[email protected]>
Shorten a format string ("%1ud" -> "%ud").

Signed-off-by: Kang-Che Sung <[email protected]>
Merge two xSnprintf() calls within the function. When the time value to
print is less than one second, the "%.u" format allows us to print no
digits in the seconds field.

Signed-off-by: Kang-Che Sung <[email protected]>
Add a new format: "1.0000ms" - "9.9999ms".
Previously they would print as ".001000s" and ".009999s", and this new
format adds one extra digit of precision.
Note that I print the unit as "ms" rather than microseconds; this saves
code for deciding whether to print the Greek "mu" or the Latin "u".

Row_printNanoseconds() now prints this list of formats:
"     0ns", "999999ns", "1.0000ms", "9.9999ms", ".010000s", ".999999s",
"1.00000s", "59.9999s", "1:00.000", "9:59.999", "10:00.00" ...

Signed-off-by: Kang-Che Sung <[email protected]>
Copy link
Member

@BenBE BenBE left a comment

Choose a reason for hiding this comment

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

LGTM. Although a bit unconventional to skip from ns to ms right away … Also, even though we have Unicode, for units I'd still prefer to use us unconditionally.

@Explorer09
Copy link
Contributor Author

LGTM. Although a bit unconventional to skip from ns to ms right away … Also, even though we have Unicode, for units I'd still prefer to use us unconditionally.

"1.0000ms" vs. "1000.0us" ...

These are same precision for showing the fractions of a second. I don't know which one looks better.

Note that the time units in the GPUMeter would have to use us anyway (due to shorter width of the field). I have thought of adding the code to display μs there.

@BenBE
Copy link
Member

BenBE commented Jan 3, 2025

I'm neutral either way …

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