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.

Sign up for GitHub

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

[not ready] Refactor utilities and respective call sites #1661

Merged
merged 4 commits into from
Sep 29, 2015
Merged

[not ready] Refactor utilities and respective call sites #1661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
98b7e0a
Refactor bearing implementation.
daniel-j-h Sep 8, 2015
31cf8a8
Remove Boost.Filesystem v3 fix for Boost < 1.48, refactor call sites.
daniel-j-h Sep 8, 2015
f9f0ffb
Remove hand written conversion code and replace with stdlib features.
daniel-j-h Sep 9, 2015
5a25741
Completely rip out Boost's Spirit / Karma for casting.
daniel-j-h Sep 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion benchmarks/static_rtree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../data_structures/query_node.hpp"
#include "../data_structures/shared_memory_vector_wrapper.hpp"
#include "../data_structures/static_rtree.hpp"
#include "../util/boost_filesystem_2_fix.hpp"
#include "../data_structures/edge_based_node.hpp"

#include <osrm/coordinate.hpp>
Expand Down
3 changes: 1 addition & 2 deletions datastore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "server/data_structures/datafacade_base.hpp"
#include "server/data_structures/shared_datatype.hpp"
#include "server/data_structures/shared_barriers.hpp"
#include "util/boost_filesystem_2_fix.hpp"
#include "util/datastore_options.hpp"
#include "util/simple_logger.hpp"
#include "util/osrm_exception.hpp"
Expand Down Expand Up @@ -185,7 +184,7 @@ int main(const int argc, const char *argv[])
BOOST_ASSERT(server_paths.end() != paths_iterator);
BOOST_ASSERT(!paths_iterator->second.empty());
const boost::filesystem::path index_file_path_absolute =
boost::filesystem::portable_canonical(paths_iterator->second);
boost::filesystem::canonical(paths_iterator->second);
const std::string &file_index_path = index_file_path_absolute.string();
paths_iterator = server_paths.find("nodesdata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
Expand Down
14 changes: 8 additions & 6 deletions descriptors/json_descriptor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../data_structures/segment_information.hpp"
#include "../data_structures/turn_instructions.hpp"
#include "../util/bearing.hpp"
#include "../util/cast.hpp"
#include "../util/integer_range.hpp"
#include "../util/json_renderer.hpp"
#include "../util/simple_logger.hpp"
Expand All @@ -44,6 +45,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <osrm/json_container.hpp>

#include <algorithm>
#include <string>

template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<DataFacadeT>
{
Expand Down Expand Up @@ -327,18 +329,18 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
std::string current_turn_instruction;
if (TurnInstruction::LeaveRoundAbout == current_instruction)
{
temp_instruction = cast::integral_to_string(
temp_instruction = std::to_string(
cast::enum_to_underlying(TurnInstruction::EnterRoundAbout));
current_turn_instruction += temp_instruction;
current_turn_instruction += "-";
temp_instruction = cast::integral_to_string(round_about.leave_at_exit + 1);
temp_instruction = std::to_string(round_about.leave_at_exit + 1);
current_turn_instruction += temp_instruction;
round_about.leave_at_exit = 0;
}
else
{
temp_instruction =
cast::integral_to_string(cast::enum_to_underlying(current_instruction));
std::to_string(cast::enum_to_underlying(current_instruction));
current_turn_instruction += temp_instruction;
}
json_instruction_row.values.push_back(current_turn_instruction);
Expand All @@ -348,7 +350,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
json_instruction_row.values.push_back(necessary_segments_running_index);
json_instruction_row.values.push_back(std::round(segment.duration / 10.));
json_instruction_row.values.push_back(
cast::integral_to_string(static_cast<unsigned>(segment.length)) + "m");
std::to_string(static_cast<unsigned>(segment.length)) + "m");
const double bearing_value = (segment.bearing / 10.);
json_instruction_row.values.push_back(bearing::get(bearing_value));
json_instruction_row.values.push_back(
Expand All @@ -372,8 +374,8 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
}

osrm::json::Array json_last_instruction_row;
temp_instruction = cast::integral_to_string(
cast::enum_to_underlying(TurnInstruction::ReachedYourDestination));
temp_instruction =
std::to_string(cast::enum_to_underlying(TurnInstruction::ReachedYourDestination));
json_last_instruction_row.values.push_back(temp_instruction);
json_last_instruction_row.values.push_back("");
json_last_instruction_row.values.push_back(0);
Expand Down
13 changes: 7 additions & 6 deletions extractor/extraction_helper_functions.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <boost/regex.hpp>

#include <limits>
#include <string>

bool simple_duration_is_valid(const std::string &s)
{
Expand Down Expand Up @@ -89,18 +90,18 @@ unsigned parseDuration(const std::string &s)
{
if (1 == result.size())
{
minutes = cast::string_to_int(result[0]);
minutes = std::stoul(result[0]);
}
if (2 == result.size())
{
minutes = cast::string_to_int(result[1]);
hours = cast::string_to_int(result[0]);
minutes = std::stoul(result[1]);
hours = std::stoul(result[0]);
}
if (3 == result.size())
{
seconds = cast::string_to_int(result[2]);
minutes = cast::string_to_int(result[1]);
hours = cast::string_to_int(result[0]);
seconds = std::stoul(result[2]);
minutes = std::stoul(result[1]);
hours = std::stoul(result[0]);
}
return (3600 * hours + 60 * minutes + seconds);
}
Expand Down
9 changes: 4 additions & 5 deletions plugins/hello_world.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#include "plugin_base.hpp"

#include "../util/cast.hpp"
#include "../util/json_renderer.hpp"

#include <osrm/json_container.hpp>
Expand All @@ -53,10 +52,10 @@ class HelloWorldPlugin final : public BasePlugin
std::string temp_string;
json_result.values["title"] = "Hello World";

temp_string = cast::integral_to_string(routeParameters.zoom_level);
temp_string = std::to_string(routeParameters.zoom_level);
json_result.values["zoom_level"] = temp_string;

temp_string = cast::integral_to_string(routeParameters.check_sum);
temp_string = std::to_string(routeParameters.check_sum);
json_result.values["check_sum"] = temp_string;
json_result.values["instructions"] = (routeParameters.print_instructions ? "yes" : "no");
json_result.values["geometry"] = (routeParameters.geometry ? "yes" : "no");
Expand All @@ -68,7 +67,7 @@ class HelloWorldPlugin final : public BasePlugin
(!routeParameters.jsonp_parameter.empty() ? "yes" : "no");
json_result.values["language"] = (!routeParameters.language.empty() ? "yes" : "no");

temp_string = cast::integral_to_string(routeParameters.coordinates.size());
temp_string = std::to_string(routeParameters.coordinates.size());
json_result.values["location_count"] = temp_string;

osrm::json::Array json_locations;
Expand All @@ -82,7 +81,7 @@ class HelloWorldPlugin final : public BasePlugin
static_cast<double>(coordinate.lat / COORDINATE_PRECISION));
json_coordinates.values.push_back(
static_cast<double>(coordinate.lon / COORDINATE_PRECISION));
json_location.values[cast::integral_to_string(counter)] = json_coordinates;
json_location.values[std::to_string(counter)] = json_coordinates;
json_locations.values.push_back(json_location);
++counter;
}
Expand Down
103 changes: 26 additions & 77 deletions server/data_structures/internal_datafacade.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../../data_structures/static_graph.hpp"
#include "../../data_structures/static_rtree.hpp"
#include "../../data_structures/range_table.hpp"
#include "../../util/boost_filesystem_2_fix.hpp"
#include "../../util/graph_loader.hpp"
#include "../../util/simple_logger.hpp"

Expand Down Expand Up @@ -179,7 +178,7 @@ template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacad
core_stream.read((char *)&number_of_markers, sizeof(unsigned));

std::vector<char> unpacked_core_markers(number_of_markers);
core_stream.read((char *)unpacked_core_markers.data(), sizeof(char)*number_of_markers);
core_stream.read((char *)unpacked_core_markers.data(), sizeof(char) * number_of_markers);

// in this case we have nothing to do
if (number_of_markers <= 0)
Expand Down Expand Up @@ -258,90 +257,40 @@ template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacad

explicit InternalDataFacade(const ServerPaths &server_paths)
{
// generate paths of data files
if (server_paths.find("hsgrdata") == server_paths.end())
{
throw osrm::exception("no hsgr file given in ini file");
}
if (server_paths.find("ramindex") == server_paths.end())
{
throw osrm::exception("no ram index file given in ini file");
}
if (server_paths.find("fileindex") == server_paths.end())
{
throw osrm::exception("no leaf index file given in ini file");
}
if (server_paths.find("geometries") == server_paths.end())
{
throw osrm::exception("no geometries file given in ini file");
}
if (server_paths.find("nodesdata") == server_paths.end())
{
throw osrm::exception("no nodes file given in ini file");
}
if (server_paths.find("coredata") == server_paths.end())
{
throw osrm::exception("no core file given in ini file");
}
if (server_paths.find("edgesdata") == server_paths.end())
{
throw osrm::exception("no edges file given in ini file");
}
if (server_paths.find("namesdata") == server_paths.end())
// cache end iterator to quickly check .find against
const auto end_it = end(server_paths);

const auto file_for = [&server_paths, &end_it](const std::string &path)
{
throw osrm::exception("no names file given in ini file");
}
const auto it = server_paths.find(path);
if (it == end_it || !boost::filesystem::is_regular_file(it->second))
throw osrm::exception("no valid " + path + " file given in ini file");
return it->second;
};

ram_index_path = file_for("ramindex");
file_index_path = file_for("fileindex");

auto paths_iterator = server_paths.find("hsgrdata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &hsgr_path = paths_iterator->second;
paths_iterator = server_paths.find("timestamp");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &timestamp_path = paths_iterator->second;
paths_iterator = server_paths.find("ramindex");
BOOST_ASSERT(server_paths.end() != paths_iterator);
ram_index_path = paths_iterator->second;
paths_iterator = server_paths.find("fileindex");
BOOST_ASSERT(server_paths.end() != paths_iterator);
file_index_path = paths_iterator->second;
paths_iterator = server_paths.find("nodesdata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &nodes_data_path = paths_iterator->second;
paths_iterator = server_paths.find("edgesdata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &edges_data_path = paths_iterator->second;
paths_iterator = server_paths.find("namesdata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &names_data_path = paths_iterator->second;
paths_iterator = server_paths.find("geometries");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &geometries_path = paths_iterator->second;
paths_iterator = server_paths.find("coredata");
BOOST_ASSERT(server_paths.end() != paths_iterator);
const boost::filesystem::path &core_data_path = paths_iterator->second;

// load data
SimpleLogger().Write() << "loading graph data";
AssertPathExists(hsgr_path);
LoadGraph(hsgr_path);
LoadGraph(file_for("hsgrdata"));

SimpleLogger().Write() << "loading edge information";
AssertPathExists(nodes_data_path);
AssertPathExists(edges_data_path);
LoadNodeAndEdgeInformation(nodes_data_path, edges_data_path);
LoadNodeAndEdgeInformation(file_for("nodesdata"), file_for("edgesdata"));

SimpleLogger().Write() << "loading core information";
AssertPathExists(core_data_path);
LoadCoreInformation(core_data_path);
LoadCoreInformation(file_for("coredata"));

SimpleLogger().Write() << "loading geometries";
AssertPathExists(geometries_path);
LoadGeometries(geometries_path);
LoadGeometries(file_for("geometries"));

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
Copy link
Member Author

Choose a reason for hiding this comment

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

/cc @TheMarex ^

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.


SimpleLogger().Write() << "loading timestamp";
LoadTimestamp(timestamp_path);
LoadTimestamp(file_for("timestamp"));

SimpleLogger().Write() << "loading street names";
AssertPathExists(names_data_path);
LoadStreetNames(names_data_path);
LoadStreetNames(file_for("namesdata"));
}

// search graph access
Expand Down
3 changes: 1 addition & 2 deletions server/data_structures/shared_datafacade.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "../../data_structures/range_table.hpp"
#include "../../data_structures/static_graph.hpp"
#include "../../data_structures/static_rtree.hpp"
#include "../../util/boost_filesystem_2_fix.hpp"
#include "../../util/make_unique.hpp"
#include "../../util/simple_logger.hpp"

Expand Down Expand Up @@ -273,7 +272,7 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
if (!boost::filesystem::exists(file_index_path))
{
SimpleLogger().Write(logDEBUG) << "Leaf file name " << file_index_path.string();
throw osrm::exception("Could not load leaf index file."
throw osrm::exception("Could not load leaf index file. "
"Is any data loaded into shared memory?");
}

Expand Down
6 changes: 3 additions & 3 deletions server/http/reply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#include "reply.hpp"

#include "../../util/cast.hpp"
#include <string>

namespace http
{
Expand All @@ -48,7 +48,7 @@ void reply::set_size(const std::size_t size)
{
if ("Content-Length" == h.name)
{
h.value = cast::integral_to_string(size);
h.value = std::to_string(size);
}
}
}
Expand Down Expand Up @@ -95,7 +95,7 @@ reply reply::stock_reply(const reply::status_type status)
const std::string status_string = reply.status_to_string(status);
reply.content.insert(reply.content.end(), status_string.begin(), status_string.end());
reply.headers.emplace_back("Access-Control-Allow-Origin", "*");
reply.headers.emplace_back("Content-Length", cast::integral_to_string(reply.content.size()));
reply.headers.emplace_back("Content-Length", std::to_string(reply.content.size()));
reply.headers.emplace_back("Content-Type", "text/html");
return reply;
}
Expand Down
5 changes: 3 additions & 2 deletions server/request_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#include <algorithm>
#include <iostream>
#include <string>

RequestHandler::RequestHandler() : routing_machine(nullptr) {}

Expand Down Expand Up @@ -102,7 +103,7 @@ void RequestHandler::handle_request(const http::request &current_request,

json_result.values["status"] = 400;
std::string message = "Query string malformed close to position ";
message += cast::integral_to_string(position);
message += std::to_string(position);
json_result.values["status_message"] = message;
osrm::json::render(current_reply.content, json_result);
return;
Expand Down Expand Up @@ -135,7 +136,7 @@ void RequestHandler::handle_request(const http::request &current_request,

// set headers
current_reply.headers.emplace_back("Content-Length",
cast::integral_to_string(current_reply.content.size()));
std::to_string(current_reply.content.size()));
if ("gpx" == route_parameters.output_format)
{ // gpx file
osrm::json::gpx_render(current_reply.content, json_result.values["route"]);
Expand Down
4 changes: 2 additions & 2 deletions server/server.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "connection.hpp"
#include "request_handler.hpp"

#include "../util/cast.hpp"
#include "../util/integer_range.hpp"
#include "../util/simple_logger.hpp"

Expand All @@ -44,6 +43,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <memory>
#include <thread>
#include <vector>
#include <string>

class Server
{
Expand All @@ -62,7 +62,7 @@ class Server
: thread_pool_size(thread_pool_size), acceptor(io_service),
new_connection(std::make_shared<http::Connection>(io_service, request_handler))
{
const std::string port_string = cast::integral_to_string(port);
const auto port_string = std::to_string(port);

boost::asio::ip::tcp::resolver resolver(io_service);
boost::asio::ip::tcp::resolver::query query(address, port_string);
Expand Down
Loading