It's not used. I believe it was being used when `Decimal` was also
responsible for formatting the fractional part of a floating point
number. But it isn't needed any more.
I don't think we need to keep track of maximum digits separately for
signed and unsigned integer formatting. I think this might mean that
signed integer formatting uses an extra byte of stack space, but, it can
also have a sign. Unlike unsigned integer formatting. So no space is
wasted here.
We also remove the `force_sign` option, which Jiff does not use. And
overall simplify the signed integer formatting to just reuse the
unsigned integer formatting. And also get rid of the dumb
`checked_abs()` usage. Not sure what I was thinking.
Using `write!` unnecessarily when a simple `f.write_str` would do ends
up generating more LLVM lines on my Biff `cargo llvm-lines` benchmark.
This PR replaces those uses of `write!` with `Formatter::write_str`.
With the error refactor, these are no longer used. Namely, while
switching to structured errors, I took that opportunity to slim down
errors so that we are not repeating parts of the input as often.
There's only a few of these, but they seem like a distinct category
from other error types. It's also nice to do this and standarize on the
specific error message.
The other reason I wanted to do this was to distinguish it from a
possible "is configuration error" category. (e.g., Trying to round a
span to the nearest year without a relative datatime.) These could also
be seen as configuration errors, but I want them to be in a separate
category I think.
We previously only used this in the `core::fmt::Display` trait
implementation, so we just inlined it there. But we'll want to use it to
get the root of the chain for error predicates, so this commit does some
refactoring to provide a standalone internal iterator for the error
chain.
For this one, I defined the error type inline with it. This just felt
right and it does stay reasonably encapsulated.
I'll probably swing back and do the same for the time range error type?
This adds a little inlining back to help reclaim a small performance
regression in `strptime`. This improves the relevant benchmark from
70ns to 64ns on my machine.
This was an incredibly tedious and tortuous refactor. But this removes
almost all of the "create ad hoc stringly-typed errors everywhere."
This partially makes progress toward #418, but my initial impetus for
doing this was to see if I could reduce binary size and improve
compilation times. My general target was to see if I could reduce total
LLVM lines. I tested this with [Biff] using this command in the root of
the Biff repo:
```
cargo llvm-lines --profile release-lto
```
Before this change, Biff had 768,596 LLVM lines. With this change, it
has 757,331 lines. So... an improvement, but a very modest one.
What about compilation times? This does seem to translate to---also a
modest---improvement. For compiling release builds of Biff. Before:
```
$ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r'
Benchmark 1: cargo b -r
Time (mean ± σ): 7.776 s ± 0.052 s [User: 65.876 s, System: 2.621 s]
Range (min … max): 7.690 s … 7.862 s 10 runs
```
After:
```
$ hyperfine -w1 --prepare 'cargo clean' 'cargo b -r'
Benchmark 1: cargo b -r
Time (mean ± σ): 7.591 s ± 0.067 s [User: 65.686 s, System: 2.564 s]
Range (min … max): 7.504 s … 7.689 s 10 runs
```
What about dev builds? Before:
```
$ hyperfine -w1 --prepare 'cargo clean' 'cargo b'
Benchmark 1: cargo b
Time (mean ± σ): 4.074 s ± 0.022 s [User: 14.493 s, System: 1.818 s]
Range (min … max): 4.037 s … 4.099 s 10 runs
```
After:
```
$ hyperfine -w1 --prepare 'cargo clean' 'cargo b'
Benchmark 1: cargo b
Time (mean ± σ): 4.541 s ± 0.027 s [User: 15.385 s, System: 2.081 s]
Range (min … max): 4.503 s … 4.591 s 10 runs
```
Well... that's disappointing. A modest improvement to release builds,
but a fairly large regression in dev builds. Maybe it's because of the
additional hand-written impls for new structured error types? Bah.
And binary size? Normal release builds (not LTO) of Biff that were
stripped were 4,431,456 bytes before this change and 4,392,064 after.
Hopefully this will unlock other improvements to justify doing this.
Note also that this slims down a number of error messages.
[Biff]: https://github.com/BurntSushi/biff
I was playing with using this to avoid parametric polymorphism
costs, but I couldn't find my way to a concrete benefit.
Either way, this impl should exist for people that want to
use it.
Both of these functions are marked as `inline`, but they can
be quite beefy. Instead, set the inlineable implementation
as the common case, and put the rest behind a non-inlineable
function.
This reduces code size somewhat.
There are probably more things like this, but I picked
this one to test its effects. It got rid of a few LLVM
lines from an LTO-compiled binary that uses Jiff. But
not much.
This also includes `Deserialize` and `Serialize` Serde impls, deferring
to `FromStr` and `Display`, respectively.
This brings `ISOWeekDate` into parity with the other datetime types in
terms of parsing/printing.
N.B. In this commit, we start trying to make error messages a little
more consistent. So there are some changes here that impact the messages
other than ISO 8601 week date parsing.
Closes#412
It looks like `icu_time::zone::models::Full` has been deprecated. So we
need some `allow(deprecated)` to squash the warnings. And we implement a
`TryFrom` for `ZonedDateTime<Iso, TimeZoneInfo<AtTime>>` as its
replacement.
Previously, I think this would have lost some information, but it sounds
like icu4x can now figure out the right DST status without being told.
Ref https://github.com/unicode-org/icu4x/pull/6754
Ref https://github.com/unicode-org/icu4x/pull/6755
I did this after the fact. I'll hold off doing another release just for
this, since this just includes a summary and a date. The actual items
don't change.
This commit writes out the Serde helpers for unsigned durations.
This is the pay off for adding the dedicated parsers/printers for a
`std::time::Duration`. Previously, we were converting to/from a
`SignedDuration`, which fundamentally limited the range of values we
could support.
Closes#380, Ref #298
I think when I originally wrote fractional formatting, I was thinking
about using precision values greater than `9`. But that never really
panned out. And I used signed values because I used signed values
everywhere.
I think this is the last change we need to be able to feasibly add
native `std::time::Duration` support.
... so that we can print `u64` values as-is without suspicious casts.
Unsigned integer printing is also simpler, so it's plausible that it's
faster too. Although I haven't been careful about benchmarking duration
printing.
In the next commit, we will finish removing the casts by tweaking
fractional printing.
This is continued progress toward natively supporting
`std::time::Duration`. I basically didn't want to duplicate all of the
code used to print a `SignedDuration` just so that we could print a
`std::time::Duration`. Since we handle the sign before/after most of the
meat of printing, we can actually lower a `SignedDuration` to a
`std::time::Duration` and focus most of our printing logic from that.
There are some papercuts in this commit though. Notably, some casts
between `u64` and `i64`, since Jiff's lowest level integer printing
routine requires a `u64`. We'll fix that in the next commit.
Thank goodness I decided to make `SignedDuration` mimic
`std::time::Duration` exactly (except for being signed). That made this
change super easy.
This only applies to parsing into a `SignedDuration` since `i64::MIN`
isn't supported by any units on `Span` (including nanoseconds, since it
only supports `(i64::MIN+1)..=i64::MAX` to make negations infallible).
We pull out the u64 prefix parser from the friendly duration and use
that. Since that does the parsing in a single pass and is overall
generally tighter, this also leads to a nice perf improvement on ISO
8601 duration parsing:
group master new
----- ------ ---
parse/iso8601_duration/long-time/duration/jiff 1.14 27.6±0.32ns ? ?/sec 1.00 24.2±0.24ns ? ?/sec
parse/iso8601_duration/long-time/span/jiff 1.21 76.6±0.17ns ? ?/sec 1.00 63.2±0.24ns ? ?/sec
parse/iso8601_duration/long/span/jiff 2.14 101.8±0.21ns ? ?/sec 1.00 47.4±0.24ns ? ?/sec
parse/iso8601_duration/medium/span/jiff 1.71 60.3±0.12ns ? ?/sec 1.00 35.2±0.12ns ? ?/sec
parse/iso8601_duration/short/duration/jiff 1.56 12.1±0.22ns ? ?/sec 1.00 7.8±0.13ns ? ?/sec
parse/iso8601_duration/short/span/jiff 1.50 47.7±0.12ns ? ?/sec 1.00 31.9±0.08ns ? ?/sec
parse/iso8601_duration/tiny/duration/jiff 1.17 6.7±0.10ns ? ?/sec 1.00 5.7±0.03ns ? ?/sec
parse/iso8601_duration/tiny/span/jiff 1.19 33.1±0.08ns ? ?/sec 1.00 27.8±0.10ns ? ?/sec
The results above are compared against current `master`
(`211a36d5ecddf1aeb9b00648d9d5e631a5420903`).
The "before" here reflects `master`
(`211a36d5ec`) and not the previous
commit:
group master new
----- ------ ---
parse/friendly/longest-time/duration/jiff 1.29 41.9±0.69ns ? ?/sec 1.00 32.4±0.42ns ? ?/sec
parse/friendly/short/duration/jiff 1.14 15.7±0.27ns ? ?/sec 1.00 13.8±0.30ns ? ?/sec
parse/friendly/tiny/duration/jiff 1.23 8.1±0.36ns ? ?/sec 1.00 6.6±0.11ns ? ?/sec
parse/iso8601_duration/long-time/duration/jiff 1.00 27.6±0.32ns ? ?/sec 1.06 29.3±0.27ns ? ?/sec
parse/iso8601_duration/short/duration/jiff 1.05 12.1±0.22ns ? ?/sec 1.00 11.5±0.20ns ? ?/sec
parse/iso8601_duration/tiny/duration/jiff 1.00 6.7±0.10ns ? ?/sec 1.15 7.7±0.09ns ? ?/sec
There are some minor regressions, but also some wins, particularly for
the friendly format.
I mostly focused on optimizing the "turn parsed values into a
`SignedDuration`" part. I used a data dependent optimization that
quickly detects whether we can avoid error handling entirely.
We could probably optimize the general case a bit more too. But this is
enough for now.
This introduces a new internal helper type, `DurationUnits`, that is
used to parse all duration types and formats. It centralizes the logic
and trims away a lot of fat. This should reduce code size (although I
haven't checked) and also improve perf. Currently, it does significantly
improve perf for parsing longer durations and especially for `Span`. It
does however slightly regress perf for parsing shorter `SignedDuration`
values.
However, I think there are a lot of perf improvement opportunities. I'll
put those in a subsequent commit.
(We still haven't implemented `std::time::Duration` yet. But
`DurationUnits` is clearly designed with it in mind. This is one helluva
yak shave!)
This also fixes a long-standing bug where we couldn't parse
`abs(i64::MIN) secs ago`. The `DurationUnits` design was specifically
motivated (in part) by this.
