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

Nanosecond specialization for iso8601.Parse #104

Merged
merged 6 commits into from
Oct 19, 2021
Merged

Conversation

chriso
Copy link
Contributor

@chriso chriso commented Oct 15, 2021

This adds additional specializations to iso8601.Parse (#82):

name                 old time/op  new time/op   delta
Parse-4              16.9ns ± 1%   18.7ns ± 1%   +10.76%  (p=0.002 n=6+6)
ParseMilliseconds-4  18.2ns ± 0%   21.3ns ± 0%   +16.93%  (p=0.002 n=6+6)
ParseMicroseconds-4   191ns ± 0%     23ns ± 1%   -87.99%  (p=0.004 n=5+6)
ParseNanoseconds-4    197ns ± 0%     25ns ± 0%   -87.47%  (p=0.002 n=6+6)

@achille-roussel
Copy link
Contributor

ParseMicroseconds-4   191ns ± 0%   192ns ± 0%     ~     (p=0.026 n=5+6)

Should we support variable precision of the sub-second value? It seems we could easily miss opportunities to use the optimized code paths if trailing zeroes are trimmed for example.

@chriso
Copy link
Contributor Author

chriso commented Oct 17, 2021

Yeah I realized after submitting this that the stdlib (and probably elsewhere) trims those trailing zeroes. I thought when formatting a time with RFC3339Nano that you'd get back exactly 9 digits after the decimal point. It should be easy enough to generalize this to handle any digits after the decimal point.

@chriso
Copy link
Contributor Author

chriso commented Oct 19, 2021

@achille-roussel see 470c959

Copy link
Contributor

@achille-roussel achille-roussel left a comment

Choose a reason for hiding this comment

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

Looking great 👍

@chriso chriso merged commit cc0871e into master Oct 19, 2021
@chriso chriso deleted the iso8601-parse-nanos branch October 19, 2021 02:31
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.

None yet

2 participants