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

[DNM][routing] Two threads bidirectional A*. #13929

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
0fe3719
[routing] Adding isOutgoing param to methods to be able to use differ…
bykoianko Oct 27, 2020
5086915
Passing isOutgoing build fix.
bykoianko Oct 27, 2020
e9c2ea5
[routing] Adding AStarGraph::m_multithreadingReady.
bykoianko Oct 28, 2020
61c4a1d
[routing] Adding comment to explain why isOutgoing should be hardcode…
bykoianko Oct 29, 2020
d02e6c1
[routing] Adding syncronization staff to IndexGraphStarterJoints.
bykoianko Oct 29, 2020
ed25d19
Fixing comments.
bykoianko Nov 2, 2020
c917672
Removing AStarGraph::m_twoThreadsReady to prevent duplicating states.
bykoianko Nov 2, 2020
d7d8977
Adding twoThreadsReady param to IndexGraphStarter.
bykoianko Nov 3, 2020
fa12c33
Adding protection to IndexGraphLoaderImpl.
bykoianko Nov 3, 2020
0942287
Passing twoThreadsReady param to Geometry.
bykoianko Nov 5, 2020
8be16db
Adding backward cache to Geomtry.
bykoianko Nov 5, 2020
828e405
Adding test code for route building.
bykoianko Nov 5, 2020
d7bfc7b
Passing param useTwoThreads to FindPathBidirectional and fixing locke…
bykoianko Nov 5, 2020
5d38d8b
Preparing BidirectionalStepContext for two threading.
bykoianko Nov 6, 2020
14747da
Implementing two threads in bidirectional astar.
bykoianko Nov 6, 2020
804d9b9
Adding some comments
bykoianko Nov 6, 2020
e2f0b0a
Implementing params.m_onVisitedVertexCallback callback for two two th…
bykoianko Nov 8, 2020
56dfc02
Calling params.m_checkLengthCallback from two threads.
bykoianko Nov 9, 2020
596ec08
Fixes to pass integration tests without checks and using one flag in …
bykoianko Nov 9, 2020
7a8c082
Removing useTwoThreads parameter. This information is passed in start…
bykoianko Nov 9, 2020
5ccfd71
Removing unnecessary comments.
bykoianko Nov 9, 2020
30419ec
Implementing getting altitude from two threads, all routing integrati…
bykoianko Nov 11, 2020
3387733
Review fixes.
bykoianko Nov 11, 2020
53e2dd4
Review fixes
bykoianko Nov 11, 2020
27c8cde
Comment about result of investigation of possible dangling references…
bykoianko Nov 12, 2020
9d7b19a
Adding synchronization for cross mwm case.
bykoianko Nov 12, 2020
e9e847a
Correct variant of switching to one thread step in route building.
bykoianko Nov 16, 2020
c418172
Refactoring.
bykoianko Nov 16, 2020
c4b8f5d
Android thread support.
bykoianko Nov 16, 2020
6a82dc7
Refactoring.
bykoianko Nov 16, 2020
c940b29
FindPathBidirectional refactoring.
bykoianko Nov 17, 2020
f02c9c7
Removes todo which is done or duplicated
bykoianko Nov 17, 2020
2f2c227
Fixing for routing integration tests passes in debug.
bykoianko Nov 17, 2020
501b1bf
Renaming isTwoThreadsReady paramter to twoThreadsReady.
bykoianko Nov 18, 2020
e476203
Using useTwoThreads instead of twoThreadsReady in some cases. And pas…
bykoianko Nov 18, 2020
6066578
[routing] Unit tests on A* bidirectional in two threads.
bykoianko Nov 18, 2020
54cbfb1
[routing] Adding two routing integration tests on two thread A* bidir…
bykoianko Nov 19, 2020
4442df9
Removing unnecessary todo.
bykoianko Nov 19, 2020
f1413d0
[routing] Implementing two thread testing for routing consistency tests.
bykoianko Nov 26, 2020
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
2 changes: 1 addition & 1 deletion coding/file_reader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <string>

// FileReader, cheap to copy, not thread safe.
// It is assumed that file is not modified during FireReader lifetime,
// It is assumed that file is not modified during FileReader lifetime,
// because of caching and assumption that Size() is constant.
class FileReader : public ModelReader
{
Expand Down
4 changes: 2 additions & 2 deletions generator/generator_tests/altitude_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,13 @@ void TestAltitudes(DataSource const & dataSource, MwmSet::MwmId const & mwmId,
std::string const & mwmPath, bool hasAltitudeExpected,
AltitudeGetter & expectedAltitudes)
{
AltitudeLoader loader(dataSource, mwmId);
AltitudeLoader loader(dataSource, mwmId, false /* twoThreadsReady */);
TEST_EQUAL(loader.HasAltitudes(), hasAltitudeExpected, ());

auto processor = [&expectedAltitudes, &loader](FeatureType & f, uint32_t const & id) {
f.ParseGeometry(FeatureType::BEST_GEOMETRY);
size_t const pointsCount = f.GetPointsCount();
geometry::Altitudes const altitudes = loader.GetAltitudes(id, pointsCount);
geometry::Altitudes const altitudes = loader.GetAltitudes(id, pointsCount, true /* isOutgoing */);

if (!routing::IsRoad(feature::TypesHolder(f)))
{
Expand Down
5 changes: 3 additions & 2 deletions generator/generator_tests_support/routing_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ void ReEncodeOsmIdsToFeatureIdsMapping(std::string const & mappingContent, std::

namespace routing
{
void TestGeometryLoader::Load(uint32_t featureId, RoadGeometry & road)
void TestGeometryLoader::Load(uint32_t featureId, RoadGeometry & road, bool isOutgoing)
{
CHECK(isOutgoing, ("TestGeometryLoader() is not ready for two threads feature parsing."));
auto const it = m_roads.find(featureId);
if (it == m_roads.cend())
return;
Expand Down Expand Up @@ -98,7 +99,7 @@ std::unique_ptr<IndexGraph> BuildIndexGraph(std::unique_ptr<TestGeometryLoader>
std::vector<Joint> const & joints)
{
auto graph = std::make_unique<IndexGraph>(std::make_shared<Geometry>(std::move(geometryLoader)),
estimator);
estimator);
graph->Import(joints);
return graph;
}
Expand Down
2 changes: 1 addition & 1 deletion generator/generator_tests_support/routing_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class TestGeometryLoader : public GeometryLoader
{
public:
// GeometryLoader overrides:
void Load(uint32_t featureId, routing::RoadGeometry & road) override;
void Load(uint32_t featureId, routing::RoadGeometry & road, bool isOutgoing) override;

void AddRoad(uint32_t featureId, bool oneWay, float speed,
routing::RoadGeometry::Points const & points);
Expand Down
10 changes: 6 additions & 4 deletions generator/restriction_collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ bool RestrictionCollector::ParseRestrictions(std::string const & path)
Joint::Id RestrictionCollector::GetFirstCommonJoint(uint32_t firstFeatureId,
uint32_t secondFeatureId) const
{
uint32_t const firstLen = m_indexGraph->GetGeometry().GetRoad(firstFeatureId).GetPointsCount();
uint32_t const secondLen = m_indexGraph->GetGeometry().GetRoad(secondFeatureId).GetPointsCount();
uint32_t const firstLen =
m_indexGraph->GetGeometry().GetRoad(firstFeatureId, true /* isOutgoing */).GetPointsCount();
uint32_t const secondLen =
m_indexGraph->GetGeometry().GetRoad(secondFeatureId, true /* isOutgoing */).GetPointsCount();

auto const firstRoad = m_indexGraph->GetRoad(firstFeatureId);
auto const secondRoad = m_indexGraph->GetRoad(secondFeatureId);
Expand All @@ -155,7 +157,7 @@ bool RestrictionCollector::FeatureHasPointWithCoords(uint32_t featureId,
m2::PointD const & coords) const
{
CHECK(m_indexGraph, ());
auto const & roadGeometry = m_indexGraph->GetGeometry().GetRoad(featureId);
auto const & roadGeometry = m_indexGraph->GetGeometry().GetRoad(featureId, true /* isOutgoing */);
uint32_t const pointsCount = roadGeometry.GetPointsCount();
for (uint32_t i = 0; i < pointsCount; ++i)
{
Expand Down Expand Up @@ -245,7 +247,7 @@ bool RestrictionCollector::CheckAndProcessUTurn(Restriction::Type & restrictionT

uint32_t & featureId = featureIds.back();

auto const & road = m_indexGraph->GetGeometry().GetRoad(featureId);
auto const & road = m_indexGraph->GetGeometry().GetRoad(featureId, true /* isOutgoing */);
// Can not do UTurn from feature to the same feature if it is one way.
if (road.IsOneWay())
return false;
Expand Down
18 changes: 10 additions & 8 deletions generator/routing_index_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class IndexGraphWrapper final
routing::Segment GetFinishSegment() const { return {}; }
bool ConvertToReal(routing::Segment const & /* segment */) const { return false; }
routing::RouteWeight HeuristicCostEstimate(routing::Segment const & /* from */,
ms::LatLon const & /* to */)
ms::LatLon const & /* to */, bool /* isOutgoing */)
{
CHECK(false, ("This method exists only for compatibility with IndexGraphStarterJoints"));
return routing::GetAStarWeightZero<routing::RouteWeight>();
Expand All @@ -200,11 +200,12 @@ class IndexGraphWrapper final
}

routing::RouteWeight GetAStarWeightEpsilon() { return routing::RouteWeight(0.0); }
bool IsTwoThreadsReady() const { return false; }
// @}

ms::LatLon const & GetPoint(routing::Segment const & s, bool forward)
ms::LatLon const & GetPoint(routing::Segment const & s, bool forward, bool isOutgoing)
{
return m_graph.GetPoint(s, forward);
return m_graph.GetPoint(s, forward, isOutgoing);
}

void GetEdgesList(routing::Segment const & child, bool isOutgoing,
Expand All @@ -223,14 +224,14 @@ class IndexGraphWrapper final
*m_AStarParents);
}

bool IsJoint(routing::Segment const & segment, bool fromStart) const
bool IsJoint(routing::Segment const & segment, bool fromStart, bool isOutgoing) const
{
return IsJointOrEnd(segment, fromStart);
return IsJointOrEnd(segment, fromStart, isOutgoing);
}

bool IsJointOrEnd(routing::Segment const & segment, bool fromStart) const
bool IsJointOrEnd(routing::Segment const & segment, bool fromStart, bool isOutgoing) const
{
return m_graph.IsJointOrEnd(segment, fromStart);
return m_graph.IsJointOrEnd(segment, fromStart, isOutgoing);
}

template <typename Vertex>
Expand All @@ -256,7 +257,8 @@ class DijkstraWrapperJoints : public routing::IndexGraphStarterJoints<IndexGraph
{
}

Weight HeuristicCostEstimate(Vertex const & /* from */, Vertex const & /* to */) override
Weight HeuristicCostEstimate(Vertex const & /* from */, Vertex const & /* to */,
bool /* isOutgoing */) override
{
return routing::GetAStarWeightZero<Weight>();
}
Expand Down
53 changes: 37 additions & 16 deletions indexer/altitude_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,13 @@ void LoadAndMap(size_t dataSize, ReaderSource<FilesContainerR::TReader> & src, T

namespace feature
{
AltitudeLoader::AltitudeLoader(DataSource const & dataSource, MwmSet::MwmId const & mwmId)
AltitudeLoader::AltitudeLoader(DataSource const & dataSource, MwmSet::MwmId const & mwmId,
bool twoThreadsReady)
: m_handle(dataSource.GetMwmHandleById(mwmId))
, m_handleBwd(twoThreadsReady ? make_unique<MwmSet::MwmHandle>(dataSource.GetMwmHandleById(mwmId))
: nullptr)
{
if (!m_handle.IsAlive())
if (!m_handle.IsAlive() || (IsTwoThreadsReady() && !m_handleBwd->IsAlive()))
return;

auto const & mwmValue = *m_handle.GetValue();
Expand All @@ -58,6 +61,12 @@ AltitudeLoader::AltitudeLoader(DataSource const & dataSource, MwmSet::MwmId cons
LoadAndMap(m_header.GetAltitudeAvailabilitySize(), src, m_altitudeAvailability,
m_altitudeAvailabilityRegion);
LoadAndMap(m_header.GetFeatureTableSize(), src, m_featureTable, m_featureTableRegion);

if (IsTwoThreadsReady())
{
m_readerBwd = make_unique<FilesContainerR::TReader>(
m_handleBwd->GetValue()->m_cont.GetReader(ALTITUDES_FILE_TAG));
}
}
catch (Reader::OpenException const & e)
{
Expand All @@ -71,24 +80,36 @@ bool AltitudeLoader::HasAltitudes() const
return m_reader != nullptr && m_header.m_minAltitude != geometry::kInvalidAltitude;
}

geometry::Altitudes const & AltitudeLoader::GetAltitudes(uint32_t featureId, size_t pointCount)
geometry::Altitudes const & AltitudeLoader::GetAltitudes(uint32_t featureId, size_t pointCount,
bool isOutgoing)
{
// Note. The backward reader and cache should not be used in case of calling GetAltitudes()
// method from one thread.
auto isForward = IsTwoThreadsReady() ? isOutgoing : true;
return GetAltitudes(featureId, pointCount, isForward ? m_reader : m_readerBwd,
isForward ? m_cache : m_cacheBwd);
}

geometry::Altitudes const & AltitudeLoader::AltitudeLoader::GetAltitudes(
uint32_t featureId, size_t pointCount, unique_ptr<FilesContainerR::TReader> & reader,
map<uint32_t, geometry::Altitudes> & cache) const
{
if (!HasAltitudes())
{
// There's no altitude section in mwm.
return m_cache
return cache
.insert(
make_pair(featureId, geometry::Altitudes(pointCount, geometry::kDefaultAltitudeMeters)))
.first->second;
}

auto const it = m_cache.find(featureId);
if (it != m_cache.end())
auto const it = cache.find(featureId);
if (it != cache.end())
return it->second;

if (!m_altitudeAvailability[featureId])
{
return m_cache
return cache
.insert(make_pair(featureId, geometry::Altitudes(pointCount, m_header.m_minAltitude)))
.first->second;
}
Expand All @@ -99,35 +120,35 @@ geometry::Altitudes const & AltitudeLoader::GetAltitudes(uint32_t featureId, siz
CHECK_LESS_OR_EQUAL(offset, m_featureTable.size(), ("Feature Id", featureId, "of", m_countryFileName));

uint64_t const altitudeInfoOffsetInSection = m_header.m_altitudesOffset + offset;
CHECK_LESS(altitudeInfoOffsetInSection, m_reader->Size(), ("Feature Id", featureId, "of", m_countryFileName));
CHECK_LESS(altitudeInfoOffsetInSection, reader->Size(), ("Feature Id", featureId, "of", m_countryFileName));

try
{
Altitudes altitudes;
ReaderSource<FilesContainerR::TReader> src(*m_reader);
ReaderSource<FilesContainerR::TReader> src(*reader);
src.Skip(altitudeInfoOffsetInSection);
bool const isDeserialized = altitudes.Deserialize(m_header.m_minAltitude, pointCount,
m_countryFileName, featureId, src);

bool const allValid =
isDeserialized &&
none_of(altitudes.m_altitudes.begin(), altitudes.m_altitudes.end(),
[](geometry::Altitude a) { return a == geometry::kInvalidAltitude; });
none_of(altitudes.m_altitudes.begin(), altitudes.m_altitudes.end(),
[](geometry::Altitude a) { return a == geometry::kInvalidAltitude; });
if (!allValid)
{
LOG(LERROR, ("Only a part point of a feature has a valid altitdue. Altitudes: ", altitudes.m_altitudes,
". Feature Id", featureId, "of", m_countryFileName));
return m_cache
LOG(LERROR, ("Only a part point of a feature has a valid altitude. Altitudes: ", altitudes.m_altitudes,
". Feature Id", featureId, "of", m_countryFileName));
return cache
.insert(make_pair(featureId, geometry::Altitudes(pointCount, m_header.m_minAltitude)))
.first->second;
}

return m_cache.insert(make_pair(featureId, move(altitudes.m_altitudes))).first->second;
return cache.insert(make_pair(featureId, move(altitudes.m_altitudes))).first->second;
}
catch (Reader::OpenException const & e)
{
LOG(LERROR, ("Feature Id", featureId, "of", m_countryFileName, ". Error while getting altitude data:", e.Msg()));
return m_cache
return cache
.insert(make_pair(featureId, geometry::Altitudes(pointCount, m_header.m_minAltitude)))
.first->second;
}
Expand Down
24 changes: 20 additions & 4 deletions indexer/altitude_loader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,43 @@ namespace feature
class AltitudeLoader
{
public:
AltitudeLoader(DataSource const & dataSource, MwmSet::MwmId const & mwmId);
AltitudeLoader(DataSource const & dataSource, MwmSet::MwmId const & mwmId,
bool twoThreadsReady);

bool HasAltitudes() const;

/// \returns altitude of feature with |featureId|. All items of the returned vector are valid
/// or the returned vector is empty.
geometry::Altitudes const & GetAltitudes(uint32_t featureId, size_t pointCount);
geometry::Altitudes const & GetAltitudes(uint32_t featureId, size_t pointCount, bool isOutgoing);

bool HasAltitudes() const;
bool IsTwoThreadsReady() const { return m_handleBwd != nullptr; }

void ClearCache() { m_cache.clear(); }
void ClearCache(bool isOutgoing) { isOutgoing ? m_cache.clear() : m_cacheBwd.clear(); }

private:
geometry::Altitudes const & GetAltitudes(
uint32_t featureId, size_t pointCount, std::unique_ptr<FilesContainerR::TReader> & reader,
std::map<uint32_t, geometry::Altitudes> & cache) const;

std::unique_ptr<CopiedMemoryRegion> m_altitudeAvailabilityRegion;
std::unique_ptr<CopiedMemoryRegion> m_featureTableRegion;

succinct::rs_bit_vector m_altitudeAvailability;
succinct::elias_fano m_featureTable;

// Note. If |twoThreadsReady| parameter of constructor is true method GetAltitudes() may be called
// from two different threads. In that case all calls of GetAltitudes() with |isOutgoing| == true
// should be done from one thread and from another one of |isOutgoing| == true.
Copy link
Contributor

Choose a reason for hiding this comment

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

// In that case all calls of GetAltitudes() with different values
// of |isOutgoing| (true or false) must be made from different threads
// and never mixed up.

// To call GetAltitudes() from two threads without locks it's necessary to have
// two caches for reading from disk (in m_handle/m_reader and m_handleBwd/m_readerBwd)
// and two caches for keeping read altitudes (m_cache and m_cacheBwd).
std::unique_ptr<FilesContainerR::TReader> m_reader;
std::unique_ptr<FilesContainerR::TReader> m_readerBwd;
std::map<uint32_t, geometry::Altitudes> m_cache;
std::map<uint32_t, geometry::Altitudes> m_cacheBwd;
AltitudeHeader m_header;
std::string m_countryFileName;
MwmSet::MwmHandle m_handle;
std::unique_ptr<MwmSet::MwmHandle> m_handleBwd;
};
} // namespace feature
2 changes: 1 addition & 1 deletion routing/async_router.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ void AsyncRouter::CalculateRoute()
absentFetcher->GenerateRequest(checkpoints);

// Run basic request.
code = router->CalculateRoute(checkpoints, startDirection, adjustToPrevRoute,
code = router->CalculateRoute(checkpoints, startDirection, true /* useTwoThreads */, adjustToPrevRoute,
delegateProxy->GetDelegate(), *route);
router->SetGuides({});
elapsedSec = timer.ElapsedSeconds(); // routing time
Expand Down
Loading