[not ready] Refactor utilities and respective call sites #1661
Conversation
| SimpleLogger().Write() << "loading r-tree"; | ||
| AssertPathExists(ram_index_path); | ||
| AssertPathExists(file_index_path); | ||
| // XXX(daniel-j-h): it says this ^ but doesn't load the rtree here |
There was a problem hiding this comment.
The StaticRTree is a thread specific pointer that is initialized on demand the first time it is used. It needs to be thread specific because it contains a stream object which is not thread safe. Which make me think we should maybe explore mmap, since virtual memory accesses is thread safe.
There was a problem hiding this comment.
It needs to be thread specific because it contains a stream object which is not thread safe.
Oh, if it's just for the stream's thread safety, there's Herb Sutter's monitor<T> and concurrent<T> (blocking and non-blocking, respectively --- this is exactly what you get with Clojure's Refs and Agents), that make it very easy:
monitor<std::ostream&> out{std::cout};
Boom, global thread safe everything-synchronization that blocks (stream in this case). Usage:
out([](auto& stream) { stream << "Hello"; });
Change monitor with concurrent, boom global thread safe everything-synchronization that does not block. Call site stays the same.
Check out his talk here if you have some time. High quality content.
|
The last commit rips out all hand-written conversion code. Please read the commit message for details. Right now there is at least one cucumber test that is failing, because of the Here is a small self contained test case: And I/O manipulation flag reference: If I would add the fixed format flag it would always contain trailing zeros specified with precision, but it seems like we do not actually want this? Why? What's the Number spec we're coding against here? I checked the JSON spec but it happily accepts trailing zeros. Except for this small issue it Just Works (tm) --- after some hours :P. |
|
@daniel-j-h I think the problem is that JSON (and javascript) in general does not have integers. So our internal JSON representation will cast everything to double values for the number type. To emphasis in a response that this value is not meant to be a floating point value we remove the |
|
@daniel-j-h the only blocker here is the added |
- removes `noexcept` specifier as we can not guarantee for not throwing - uses a namespace instead of a struct + static function combination - asserts for heading degree in [0, 360] range (both sides inclusive!) - header only since implementation does not hide anything - adds `inline` specifier as compiler hint
We needed this for Boost < 1.48, but per our Wiki on building OSRM: > On Ubuntu 12.04 you will be limited to OSRM tag v0.3.10 because > later versions **require Boost v1.49+** and installing this > causes problems with libluabind-dev package. Thus, rip it out! To keep the commits atomic and isolated, I also refactored all call sites that used the functionality from the portability fix. While doing this, I also simplified the monster of around ~100 lines of file path checking --- lambda's are awesome' use them! References: - http://stackoverflow.com/a/1750710 - https://github.com/Project-OSRM/osrm-backend/wiki/Building-on-Ubuntu
With C++11 the stdlib gains:
- `std::stoi` function family to convert from `std::string` to integral type
- `std::to_string` to convert from number types to `std::string`
The only reason for hand-writing the conversion code therefore is
performance. I benchmarked an `osrm-extract` with the hand-written code
against one with the stdlib conversion features and could not find any
significant difference (we switch back and forth between C++ and Lua,
shaving off a few us in conversion doesn't gain us much).
Formatting arithmetic types in the default format with given precision
requires streams, but is doable in a few lines of idiomatic stdlib code.
For this, there is now the following function template available:
template <Arithmetic T, int Precision = 6>
inline std::string to_string_with_precision(const T);
that requires integral or floating point types and returns a formatted
string in the defaukt format with the given precision applied.
In addition this completely rips out Boost.Spirit from the `casts.hpp`
header, resulting in faster compile times.
Boom!
References:
- http://en.cppreference.com/w/cpp/string/basic_string/stol
- http://en.cppreference.com/w/cpp/string/basic_string/to_string
- http://www.kumobius.com/2013/08/c-string-to-int/
This rips out the Bost Spirit / Karma conversion code, using the stdlib
and lightweight alternatives instead.
The main benefit is an immense decrease in compilation times, for every
translation unit that requires the `util/cast.hpp` header.
Note: compared to the version before, there is a minor change in
behavior: the double `-0` was printed as `0` before and is now printed
as `-0`. This comes from the IEE754 standard, specifying signed zeros,
that is `+0` and `-0`. Interesting for us: JavaScript uses IEE754,
resulting in no breakage if used in arithmetic.
Small test case, left hand side was before, right hand side is now:
$ ./a.out
-1.123457 vs -1.123457
-1 vs -1
-1.3 vs -1.3
0 vs -0
0 vs 0
0 vs 0
1.3 vs 1.3
1.123457 vs 1.123457
References:
- https://en.wikipedia.org/wiki/Signed_zero
- http://www.boost.org/doc/libs/1_59_0/doc/html/boost/algorithm/trim_right_if.html
- http://www.boost.org/doc/libs/1_59_0/doc/html/boost/algorithm/is_any_of.html
daniel-j-h
force-pushed
the
refactor_util
branch
from
3fe4d26 to
5a25741
Compare
This Pull Request tracks the progress. Do not merge yet.
The high level goal is to improve the utilities in regard to warnings, build times, compiler and linker specific settings and features.
Please read every commit's description, as they have detailed explanations attached. This pull request just tracks the overall progress.