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

Do not generate intermediate .osrm file in osrm-extract. #6354

Merged
merged 17 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
14 changes: 13 additions & 1 deletion .github/workflows/osrm-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,19 @@ jobs:
# when `--memory-swap` value equals `--memory` it means container won't use swap
# see https://docs.docker.com/config/containers/resource_constraints/#--memory-swap-details
MEMORY_ARGS="--memory=1g --memory-swap=1g"
docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-extract -p /opt/car.lua /data/berlin-latest.osm.pbf
docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-extract --dump-nbg-graph -p /opt/car.lua /data/berlin-latest.osm.pbf
docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-components /data/berlin-latest.osrm /data/berlin-latest.geojson
Copy link
Member Author

Choose a reason for hiding this comment

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

Added kind of smoke test for osrm-components.

if [ ! -s "${PWD}/berlin-latest.geojson" ]
then
>&2 echo "No berlin-latest.geojson found"
exit 1
fi

# here we check that `osrm-partition` accepts base file path with `.osrm` extension even if `.osrm` file doesn't exist
Copy link
Member

Choose a reason for hiding this comment

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

I noticed the command args refer to the .osrm file. We probably want to change the wording there too.

"input,i",
boost::program_options::value<boost::filesystem::path>(&customization_config.base_path),
"Input file in .osrm format");

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated it

# this way we check backward compatibility with existing pipelines which may rely on it
rm -rf "${PWD}/berlin-latest.osrm"
docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-partition /data/berlin-latest.osrm

docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-partition /data/berlin-latest.osrm
docker run $MEMORY_ARGS -t -v "${PWD}:/data" "${TAG}" osrm-customize /data/berlin-latest.osrm
docker run $MEMORY_ARGS --name=osrm-container -t -p 5000:5000 -v "${PWD}:/data" "${TAG}" osrm-routed --algorithm mld /data/berlin-latest.osrm &
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@
- FIXED: Ensure u-turn exists in intersection view. [#6376](https://github.com/Project-OSRM/osrm-backend/pull/6376)
- Profile:
- CHANGED: Bicycle surface speeds [#6212](https://github.com/Project-OSRM/osrm-backend/pull/6212)

- Tools:
- CHANGED: Do not generate intermediate .osrm file in osrm-extract. [#6354](https://github.com/Project-OSRM/osrm-backend/pull/6354)

# 5.26.0
- Changes from 5.25.0
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ The flag `-v "${PWD}:/data"` creates the directory `/data` inside the docker con
docker run -t -v "${PWD}:/data" osrm/osrm-backend osrm-partition /data/berlin-latest.osrm
docker run -t -v "${PWD}:/data" osrm/osrm-backend osrm-customize /data/berlin-latest.osrm

Note that `berlin-latest.osrm` has a different file extension.
Note there is no `berlin-latest.osrm` file, but multiple `berlin-latest.osrm.*` files, i.e. `berlin-latest.osrm` is not file path, but "base" path referring to set of files and there is an option to omit this `.osrm` suffix completely(e.g. `osrm-partition /data/berlin-latest`).
Copy link
Member Author

Choose a reason for hiding this comment

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

@mjjbell I noticed that all our files match .osrm.* pattern, so IMO it feels as a good idea to still use .osrm extension everywhere, but consider it as "base" path, but not separate file. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, agreed.


docker run -t -i -p 5000:5000 -v "${PWD}:/data" osrm/osrm-backend osrm-routed --algorithm mld /data/berlin-latest.osrm

Expand Down
6 changes: 3 additions & 3 deletions docs/nodejs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
## OSRM

The `OSRM` method is the main constructor for creating an OSRM instance.
An OSRM instance requires a `.osrm` dataset, which is prepared by the OSRM toolchain.
You can create such a `.osrm` file by running the OSRM binaries we ship in `node_modules/osrm/lib/binding/` and default
An OSRM instance requires a `.osrm.*` dataset(`.osrm.*` because it contains several files), which is prepared by the OSRM toolchain.
You can create such a `.osrm.*` dataset by running the OSRM binaries we ship in `node_modules/osrm/lib/binding/` and default
profiles (e.g. for setting speeds and determining road types to route on) in `node_modules/osrm/profiles/`:

node_modules/osrm/lib/binding/osrm-extract data.osm.pbf -p node_modules/osrm/profiles/car.lua
node_modules/osrm/lib/binding/osrm-contract data.osrm

Consult the [osrm-backend](https://github.com/Project-OSRM/osrm-backend) documentation for further details.

Once you have a complete `network.osrm` file, you can calculate routes in javascript with this object.
Once you have a complete `network.osrm.*` dataset, you can calculate routes in javascript with this object.

```javascript
var osrm = new OSRM('network.osrm');
Expand Down
8 changes: 4 additions & 4 deletions include/extractor/extraction_containers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ class ExtractionContainers
void PrepareTrafficSignals(const ReferencedTrafficSignals &referenced_traffic_signals);
void PrepareEdges(ScriptingEnvironment &scripting_environment);

void WriteNodes(storage::tar::FileWriter &file_out) const;
void WriteEdges(storage::tar::FileWriter &file_out) const;
void WriteMetadata(storage::tar::FileWriter &file_out) const;
void WriteCharData(const std::string &file_name);

public:
Expand Down Expand Up @@ -75,6 +72,8 @@ class ExtractionContainers
std::vector<InputTrafficSignal> external_traffic_signals;
TrafficSignals internal_traffic_signals;

std::vector<NodeBasedEdge> used_edges;

// List of restrictions (conditional and unconditional) before we transform them into the
// output types. Input containers reference OSMNodeIDs. We can only transform them to the
// correct internal IDs after we've read everything. Without a multi-parse approach,
Expand All @@ -84,11 +83,12 @@ class ExtractionContainers

std::vector<InputManeuverOverride> external_maneuver_overrides_list;
std::vector<UnresolvedManeuverOverride> internal_maneuver_overrides;
std::unordered_set<NodeID> used_barrier_nodes;
NodeVector used_nodes;

ExtractionContainers();

void PrepareData(ScriptingEnvironment &scripting_environment,
const std::string &osrm_path,
const std::string &names_data_path);
};
} // namespace extractor
Expand Down
21 changes: 16 additions & 5 deletions include/extractor/extractor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,25 @@ class Extractor
Extractor(ExtractorConfig extractor_config) : config(std::move(extractor_config)) {}
int run(ScriptingEnvironment &scripting_environment);

private:
struct ParsedOSMData
{
LaneDescriptionMap turn_lane_map;
std::vector<TurnRestriction> turn_restrictions;
std::vector<UnresolvedManeuverOverride> unresolved_maneuver_overrides;
TrafficSignals traffic_signals;
std::unordered_set<NodeID> barriers;
std::vector<util::Coordinate> osm_coordinates;
extractor::PackedOSMIDs osm_node_ids;
std::vector<NodeBasedEdge> edge_list;
std::vector<NodeBasedEdgeAnnotation> annotation_data;
};

private:
ExtractorConfig config;

std::tuple<LaneDescriptionMap,
std::vector<TurnRestriction>,
std::vector<UnresolvedManeuverOverride>,
TrafficSignals>
ParseOSMData(ScriptingEnvironment &scripting_environment, const unsigned number_of_threads);
ParsedOSMData ParseOSMData(ScriptingEnvironment &scripting_environment,
Copy link
Member Author

Choose a reason for hiding this comment

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

IMO this ParsedOSMData is a bit more readable than tuple we used before.

const unsigned number_of_threads);

EdgeID BuildEdgeExpandedGraph(
// input data
Expand Down
1 change: 1 addition & 0 deletions include/extractor/extractor_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ struct ExtractorConfig final : storage::IOConfig
bool use_metadata = false;
bool parse_conditionals = false;
bool use_locations_cache = true;
bool dump_nbg_graph = false;
};
} // namespace extractor
} // namespace osrm
Expand Down
9 changes: 2 additions & 7 deletions include/extractor/files.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,11 @@ inline void readConditionalRestrictions(const boost::filesystem::path &path,
}

// reads .osrm file which is a temporary file of osrm-extract
template <typename BarrierOutIter, typename PackedOSMIDsT>
template <typename PackedOSMIDsT>
void readRawNBGraph(const boost::filesystem::path &path,
BarrierOutIter barriers,
std::vector<util::Coordinate> &coordinates,
PackedOSMIDsT &osm_node_ids,
std::vector<extractor::NodeBasedEdge> &edge_list,
std::vector<extractor::NodeBasedEdgeAnnotation> &annotations)
std::vector<extractor::NodeBasedEdge> &edge_list)
{
const auto fingerprint = storage::tar::FileReader::VerifyFingerprint;
storage::tar::FileReader reader{path, fingerprint};
Expand All @@ -468,10 +466,7 @@ void readRawNBGraph(const boost::filesystem::path &path,
reader.ReadStreaming<extractor::QueryNode>("/extractor/nodes",
boost::make_function_output_iterator(decode));

reader.ReadStreaming<NodeID>("/extractor/barriers", barriers);

storage::serialization::read(reader, "/extractor/edges", edge_list);
storage::serialization::read(reader, "/extractor/annotations", annotations);
}

template <typename NameTableT>
Expand Down
17 changes: 10 additions & 7 deletions include/extractor/node_based_graph_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,19 @@ namespace extractor
class NodeBasedGraphFactory
{
public:
// The node-based graph factory loads the *.osrm file and transforms the data within into the
// The node-based graph factory transforms the graph data into the
// node-based graph to represent the OSM network. This includes geometry compression, annotation
// data optimisation and many other aspects. After this step, the edge-based graph factory can
// turn the graph into the routing graph to be used with the navigation algorithms.
NodeBasedGraphFactory(const boost::filesystem::path &input_file,
ScriptingEnvironment &scripting_environment,
NodeBasedGraphFactory(ScriptingEnvironment &scripting_environment,
Copy link
Member

Choose a reason for hiding this comment

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

Above comment will need updating.

std::vector<TurnRestriction> &turn_restrictions,
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
const TrafficSignals &traffic_signals);
const TrafficSignals &traffic_signals,
std::unordered_set<NodeID> &&barriers,
std::vector<util::Coordinate> &&coordinates,
extractor::PackedOSMIDs &&osm_node_ids,
const std::vector<NodeBasedEdge> &edge_list,
std::vector<NodeBasedEdgeAnnotation> &&annotation_data);

auto const &GetGraph() const { return compressed_output_graph; }
auto const &GetBarriers() const { return barriers; }
Expand All @@ -60,9 +64,8 @@ class NodeBasedGraphFactory
void ReleaseOsmNodes();

private:
// Get the information from the *.osrm file (direct product of the extractor callback/extraction
// containers) and prepare the graph creation process
void LoadDataFromFile(const boost::filesystem::path &input_file);
// Build and validate compressed output graph
void BuildCompressedOutputGraph(const std::vector<NodeBasedEdge> &edge_list);

// Compress the node-based graph into a compact representation of itself. This removes storing a
// single edge for every part of the geometry and might also combine meta-data for multiple
Expand Down
2 changes: 1 addition & 1 deletion include/partitioner/partitioner_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace partitioner
struct PartitionerConfig final : storage::IOConfig
{
PartitionerConfig()
: IOConfig({".osrm", ".osrm.fileIndex", ".osrm.ebg_nodes", ".osrm.enw"},
: IOConfig({".osrm.fileIndex", ".osrm.ebg_nodes", ".osrm.enw"},
{".osrm.hsgr", ".osrm.cnbg"},
{".osrm.ebg",
".osrm.cnbg",
Expand Down
Loading