Skip to content

Add std.fmt.formatDuration and std.fmt.duration - #7297

Merged
andrewrk merged 5 commits into
ziglang:masterfrom
jdknezek:std-fmt-formatDuration
Jan 12, 2021
Merged

Add std.fmt.formatDuration and std.fmt.duration#7297
andrewrk merged 5 commits into
ziglang:masterfrom
jdknezek:std-fmt-formatDuration

Conversation

@jdknezek

Copy link
Copy Markdown
Contributor

formatDuration works on a writer, and duration wraps a u64 to allow pleasant injection into format strings.

`formatDuration` works on a writer, and `duration` wraps a u64 to allow pleasant injection into format strings.
(bad copy/paste)
Comment threadlib/std/fmt.zig
Comment threadlib/std/fmt.zig Outdated
}) |unit| {
if (ns >= unit.ns) {
const units = @intToFloat(f64, ns_remaining) / @intToFloat(f64, unit.ns);
return format(writer, "{d}{}", .{ units, unit.sep });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

d will print the fp value with full precision, is that wanted?
Also, {s} for string formats please.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It's probably reasonable to limit the max precision to 3 decimal places (i.e. down to the next unit). I don't think there's any way to do max precision with fmt, but I can div(floor(mul))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can set the precision parameter for that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Unfortunately @as(f64, 1) formatted with {d:.3} comes out as 1.000, where I would prefer to omit unnecessary precision

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is using floats here really necessary? Since ns_remaining is a fixed-point number, you can manually split it into integral and fractional parts, and print them with formatInt.

@jdknezekjdknezekDec 4, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Is eliminating the floating point operation worth the extra complexity?

-// Floor to 3 decimal placesconstkunits=ns*1000/unit.ns;
-constunits=@intToFloat(f64, kunits) /1000.0;
-tryformatFloatDecimal(units, .{}, writer);
+tryformatInt(kunits/1000, 10, false, .{}, writer);
+// Write up to 3 decimal places+constfrac=kunits%1000;
+if (frac>0) {
+trywriter.writeByte('.');
+if (frac%10>0) {
+tryformatInt(frac, 10, false, .{ .fill='0', .width=3 }, writer);
+ } elseif (frac%100>0) {
+tryformatInt(frac/10, 10, false, .{ .fill='0', .width=2 }, writer);
+ } else {
+tryformatInt(frac/100, 10, false, .{ .fill='0', .width=1 }, writer);
+ }
+ }

Serious question, I don't often work in environments where one floating operation would be noticeable

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Not sure how performance compares, but another option:

constfrac=kunits%1000;
if (frac>0) {
trywriter.writeByte('.');
varfrac_buf: [3]u8=undefined;
_=formatIntBuf(&frac_buf, frac, 10, false, .{ .fill='0', .width=3 });
tryformatBuf(frac_buf[0..std.mem.indexOfScalar(u8, &frac_buf, '0') orelsefrac_buf.len], .{}, writer);
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thank you for the feedback! I have eliminated the floating point operations.

Comment threadlib/std/fmt.zig Outdated
Comment threadlib/std/fmt.zig Outdated
Comment threadlib/std/fmt.zig
@jdknezek

Copy link
Copy Markdown
ContributorAuthor

I've addressed the first pass of comments - thanks for the feedback!

@jdknezek

Copy link
Copy Markdown
ContributorAuthor

I've now eliminated the floating point operations.

@VexuVexu added the standard library This issue involves writing Zig code for the standard library. label Dec 25, 2020
@andrewrk
andrewrk merged commit fc10c9c into ziglang:masterJan 12, 2021
@andrewrk

Copy link
Copy Markdown
Member

Thanks @jdknezek! Follow-up proposal: #7752

dgbuckley pushed a commit to dgbuckley/zig that referenced this pull request Mar 9, 2021
`formatDuration` works on a writer, and `duration` wraps a u64 to allow pleasant injection into format strings.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

standard libraryThis issue involves writing Zig code for the standard library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@jdknezek@andrewrk@LemonBoy@daurnimator@zigazeljko@Vexu