-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Render floating point numbers using faster method #5188
Conversation
@danpat i'm glad to see my suggestion worked out! i knew we could count on the rapidjson authors implementation to be webscale :p |
As far as I understand, the code implements Grisu2, which produces a correct, but not always optimal result. E.g. you might get 0.21000000000000002 instead of 0.21 for certain values. From the paper at https://www.cs.tufts.edu/~nr/cs257/archive/florian-loitsch/printf.pdf:
Errol3 is a newer method, but it seems their initial speed claims were wrong: https://github.com/marcandrysco/Errol Another new method called Ryu claim to be faster than Grisu: |
3f5e961
to
f894507
Compare
@emiltin haha, more than I ever wanted to know about decimal number formatting :-) On macOS, The C99 standard, section 5.2.4.2.2 Paragraph 6 doc here says:
So given that there is no strict requirement from the standard about how these numbers come out, and all our tests are passing, I'm inclined to say that any change in the number formatting that folks experience after this PR goes out was previously undefined behaviour anyway. |
include/util/json_renderer.hpp
Outdated
@@ -34,8 +35,31 @@ struct Renderer | |||
|
|||
void operator()(const Number &number) const | |||
{ | |||
out.precision(10); | |||
out << number.value; | |||
char buffer[256] = {'\0'}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you choose 256 as the total length of the number here?
…ad of stdlib to speed up JSON generation.
253afd5
to
e072dc6
Compare
- Changes from 5.18.0: - Optimizations: - CHANGED: Use Grisu2 for serializing floating point numbers. [Project-OSRM#5188](Project-OSRM#5188) - ADDED: Node bindings can return pre-rendered JSON buffer. [Project-OSRM#5189](Project-OSRM#5189) - Profiles: - CHANGED: Bicycle profile now blacklists barriers instead of whitelisting them [Project-OSRM#5076 ](Project-OSRM#5076) - CHANGED: Foot profile now blacklists barriers instead of whitelisting them [Project-OSRM#5077 ](Project-OSRM#5077) - CHANGED: Support maxlength and maxweight in car profile [Project-OSRM#5101](Project-OSRM#5101] - Bugfixes: - FIXED: collapsing of ExitRoundabout instructions [Project-OSRM#5114](Project-OSRM#5114) - Misc: - CHANGED: Support up to 512 named shared memory regions [Project-OSRM#5185](Project-OSRM#5185)
When generating a JSON response string,
osrm-routed
is currently using whichever stdlib it's linked against to output numbers using the standardostream << some_number
method.For API responses with not too many numbers (e.g.
/viaroute
withgeometries=polyline
), this works just fine. For responses that contain large numbers of floating point numbers (e.g. very large/table
requests), the total response time can include a significant amount of time just spent turningdouble
numbers into strings.This PR replaces the dependence of
ostream operator << (double)
with a custom floating pointer-to-string renderer lifted from https://github.com/miloyip/dtoa-benchmark/blob/c4020c62754950d38a1aaaed2975b05b441d1e7d/src/milo/dtoa_milo.h.The github README for the dtoa-benchmark project claims that this code is about 20x faster than
ostringstream operator << (double)
. Local benchmarking I did pretty much confirms this.As far as I can tell, the output values are identical to the stdlib strings, so this is basically just a drop-in replacement with a free performance boost. For responses that contain lots of doubles, this speeds things along nicely.
Tasklist
Requirements / Relations
Link any requirements here. Other pull requests this PR is based on?