Skip to content

Commit 3618935

Browse files
muirdmfacebook-github-bot
authored andcommitted
common: add PathMap constructor to skip O(n*log(n)) work
Summary: Add a constructor that directly takes `vector<pair<..>>>`, only sorting the vector if necessary. This is effectively a "bulk import" for tree entries. Existing code constructs the PathMap by calling `PathMap::emplace()` in a loop, but that is inefficient since emplace does a `O(log(n))` lookup every time (even if the data is already sorted), and then potentially has to shift data around in the vector (if the data is not already sorted). I'm going to use this in upcoming diffs to optimize tree construction. Differential Revision: D81650017 fbshipit-source-id: 87a9ff11bbe60111bd165080414d63f479e958bb
1 parent 8f151ec commit 3618935

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

eden/common/utils/PathMap.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
7575
return isPathPieceLess(Piece(lhs.first), Piece(b), caseSensitive_);
7676
}
7777

78+
// Compare two stored Pairs.
79+
template <typename A, typename B>
80+
typename std::enable_if<std::is_convertible<A, Piece>::value, bool>::type
81+
operator()(const std::pair<A, B>& lhs, const std::pair<A, B>& rhs) const {
82+
return isPathPieceLess(
83+
Piece(lhs.first), Piece(rhs.first), caseSensitive_);
84+
}
85+
7886
CaseSensitivity caseSensitive_{kPathMapDefaultCaseSensitive};
7987
};
8088

@@ -123,6 +131,28 @@ class PathMap : private folly::fbvector<std::pair<Key, Value>> {
123131
}
124132
}
125133

134+
// Initialize using given vector of entries, sorting if needed.
135+
// This is more efficient than calling PathMap::emplace n times.
136+
PathMap(Vector&& entries, CaseSensitivity caseSensitive)
137+
: Vector(std::move(entries)), compare_(caseSensitive) {
138+
Vector& vec = *this;
139+
140+
// It seems like std::sort isn't guaranteed to be fast when the data is
141+
// already sorted, so let's check ourselves.
142+
143+
bool needsSort = false;
144+
for (size_t idx = 0; idx < vec.size(); idx++) {
145+
if (idx > 0 && !compare_(vec[idx - 1].first, vec[idx].first)) {
146+
needsSort = true;
147+
break;
148+
}
149+
}
150+
151+
if (needsSort) {
152+
std::sort(vec.begin(), vec.end(), compare_);
153+
}
154+
}
155+
126156
// Inherit the underlying vector copy/assignment.
127157
PathMap(const PathMap& other) : Vector(other), compare_(other.compare_) {}
128158
PathMap& operator=(const PathMap& other) {

eden/common/utils/test/PathMapTest.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,3 +340,16 @@ TEST(PathMapTest, collatePathMaps_caseSensitive) {
340340
ASSERT_EQ(expected, collatePathMaps(a, b));
341341
ASSERT_EQ(expected.getCaseSensitivity(), CaseSensitivity::Sensitive);
342342
}
343+
344+
TEST(PathMapTest, vecConstructor) {
345+
folly::fbvector<std::pair<PathComponent, bool>> vec{
346+
{PathComponent("zebra"), true},
347+
{PathComponent("aardvark"), false},
348+
};
349+
350+
auto m = PathMap<bool>{std::move(vec), CaseSensitivity::Sensitive};
351+
352+
// Make sure we can access both elements (i.e. confirm they were sorted).
353+
ASSERT_EQ(true, m.at(PathComponent{"zebra"}));
354+
ASSERT_EQ(false, m.at(PathComponent{"aardvark"}));
355+
}

0 commit comments

Comments
 (0)