Skip to content

Commit 3315681

Browse files
jviottijviotti
committed
Use a single lock in indexing dispatches
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent 4fc6247 commit 3315681

File tree

2 files changed

+100
-49
lines changed

2 files changed

+100
-49
lines changed

‎src/build/build.cc‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,36 @@ auto Build::refresh(const std::filesystem::path &path) -> void {
124124
this->entries_[path.native()].file_mark = value;
125125
}
126126

127+
auto Build::track_unsafe(const std::filesystem::path &path) -> void {
128+
assert(path.is_absolute());
129+
const auto &path_string{path.native()};
130+
const auto destination_match{this->entries_.find(path_string)};
131+
if (destination_match != this->entries_.end()) {
132+
destination_match->second.tracked.store(true, std::memory_order_relaxed);
133+
}
134+
135+
auto parent_key{path_string};
136+
while (true) {
137+
const auto slash{parent_key.rfind('/')};
138+
if (slash == std::string::npos || slash < this->root_string.size()) {
139+
break;
140+
}
141+
142+
parent_key = parent_key.substr(0, slash);
143+
const auto parent_match{this->entries_.find(parent_key)};
144+
if (parent_match == this->entries_.end()) {
145+
break;
146+
}
147+
148+
if (parent_match->second.tracked.load(std::memory_order_relaxed)) {
149+
break;
150+
}
151+
152+
parent_match->second.tracked.store(true, std::memory_order_relaxed);
153+
parent_match->second.is_directory.store(true, std::memory_order_relaxed);
154+
}
155+
}
156+
127157
auto Build::track(const std::filesystem::path &path) -> void {
128158
assert(path.is_absolute());
129159
const auto &path_string{path.native()};

‎src/build/include/sourcemeta/one/build.h‎

Lines changed: 70 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <sourcemeta/core/json.h>
99

1010
#include <array> // std::array
11+
#include <atomic> // std::atomic
1112
#include <filesystem> // std::filesystem
1213
#include <functional> // std::function, std::reference_wrapper, std::cref
1314
#include <optional> // std::optional
@@ -50,34 +51,33 @@ class SOURCEMETA_ONE_BUILD_EXPORT Build {
5051
const std::vector<std::filesystem::path> &dependencies)
5152
-> bool {
5253
const auto &destination_string{destination.native()};
53-
std::shared_lock lock{this->mutex};
54-
const auto cached_match{this->entries_.find(destination_string)};
55-
if (cached_match != this->entries_.end()) {
56-
const auto &entry{cached_match->second};
57-
if (entry.file_mark.has_value() &&
58-
(!entry.static_dependencies.empty() ||
59-
!entry.dynamic_dependencies.empty())) {
60-
bool static_dependencies_match{dependencies.size() ==
61-
entry.static_dependencies.size()};
62-
if (static_dependencies_match) {
63-
for (std::size_t index = 0; index < dependencies.size(); ++index) {
64-
if (dependencies[index] != entry.static_dependencies[index]) {
65-
static_dependencies_match = false;
66-
break;
54+
{
55+
std::shared_lock lock{this->mutex};
56+
const auto cached_match{this->entries_.find(destination_string)};
57+
if (cached_match != this->entries_.end()) {
58+
const auto &entry{cached_match->second};
59+
if (entry.file_mark.has_value() &&
60+
(!entry.static_dependencies.empty() ||
61+
!entry.dynamic_dependencies.empty())) {
62+
bool static_dependencies_match{dependencies.size() ==
63+
entry.static_dependencies.size()};
64+
if (static_dependencies_match) {
65+
for (std::size_t index = 0; index < dependencies.size(); ++index) {
66+
if (dependencies[index] != entry.static_dependencies[index]) {
67+
static_dependencies_match = false;
68+
break;
69+
}
6770
}
6871
}
69-
}
7072

71-
if (this->dispatch_is_cached(entry, static_dependencies_match)) {
72-
lock.unlock();
73-
this->track(destination);
74-
return false;
73+
if (this->dispatch_is_cached(entry, static_dependencies_match)) {
74+
this->track_unsafe(destination);
75+
return false;
76+
}
7577
}
7678
}
7779
}
7880

79-
lock.unlock();
80-
8181
Dependencies static_dependency_references;
8282
static_dependency_references.reserve(dependencies.size());
8383
for (const auto &dependency : dependencies) {
@@ -107,39 +107,38 @@ class SOURCEMETA_ONE_BUILD_EXPORT Build {
107107
const Context &context, const DependencyTypes &...dependencies)
108108
-> bool {
109109
const auto &destination_string{destination.native()};
110-
std::shared_lock lock{this->mutex};
111-
const auto cached_match{this->entries_.find(destination_string)};
112-
if (cached_match != this->entries_.end()) {
113-
const auto &entry{cached_match->second};
114-
if (entry.file_mark.has_value() &&
115-
(!entry.static_dependencies.empty() ||
116-
!entry.dynamic_dependencies.empty())) {
117-
const std::array<const std::filesystem::path *,
118-
sizeof...(DependencyTypes)>
119-
dependency_pointers{{&dependencies...}};
120-
constexpr auto dependency_count{sizeof...(DependencyTypes)};
121-
bool static_dependencies_match{entry.static_dependencies.size() ==
122-
dependency_count};
123-
if (static_dependencies_match) {
124-
for (std::size_t index = 0; index < dependency_count; ++index) {
125-
if (*dependency_pointers[index] !=
126-
entry.static_dependencies[index]) {
127-
static_dependencies_match = false;
128-
break;
110+
{
111+
std::shared_lock lock{this->mutex};
112+
const auto cached_match{this->entries_.find(destination_string)};
113+
if (cached_match != this->entries_.end()) {
114+
const auto &entry{cached_match->second};
115+
if (entry.file_mark.has_value() &&
116+
(!entry.static_dependencies.empty() ||
117+
!entry.dynamic_dependencies.empty())) {
118+
const std::array<const std::filesystem::path *,
119+
sizeof...(DependencyTypes)>
120+
dependency_pointers{{&dependencies...}};
121+
constexpr auto dependency_count{sizeof...(DependencyTypes)};
122+
bool static_dependencies_match{entry.static_dependencies.size() ==
123+
dependency_count};
124+
if (static_dependencies_match) {
125+
for (std::size_t index = 0; index < dependency_count; ++index) {
126+
if (*dependency_pointers[index] !=
127+
entry.static_dependencies[index]) {
128+
static_dependencies_match = false;
129+
break;
130+
}
129131
}
130132
}
131-
}
132133

133-
if (this->dispatch_is_cached(entry, static_dependencies_match)) {
134-
lock.unlock();
135-
this->track(destination);
136-
return false;
134+
if (this->dispatch_is_cached(entry, static_dependencies_match)) {
135+
this->track_unsafe(destination);
136+
return false;
137+
}
137138
}
138139
}
139140
}
140141

141-
lock.unlock();
142-
143142
const Dependencies static_dependency_references{std::cref(dependencies)...};
144143

145144
std::vector<std::filesystem::path> dynamic_dependencies;
@@ -163,8 +162,29 @@ class SOURCEMETA_ONE_BUILD_EXPORT Build {
163162
std::optional<mark_type> file_mark;
164163
std::vector<std::filesystem::path> static_dependencies;
165164
std::vector<std::filesystem::path> dynamic_dependencies;
166-
bool tracked{false};
167-
bool is_directory{false};
165+
std::atomic<bool> tracked{false};
166+
std::atomic<bool> is_directory{false};
167+
168+
Entry() = default;
169+
~Entry() = default;
170+
Entry(Entry &&other) noexcept
171+
: file_mark{other.file_mark},
172+
static_dependencies{std::move(other.static_dependencies)},
173+
dynamic_dependencies{std::move(other.dynamic_dependencies)},
174+
tracked{other.tracked.load(std::memory_order_relaxed)},
175+
is_directory{other.is_directory.load(std::memory_order_relaxed)} {}
176+
auto operator=(Entry &&other) noexcept -> Entry & {
177+
file_mark = other.file_mark;
178+
static_dependencies = std::move(other.static_dependencies);
179+
dynamic_dependencies = std::move(other.dynamic_dependencies);
180+
tracked.store(other.tracked.load(std::memory_order_relaxed),
181+
std::memory_order_relaxed);
182+
is_directory.store(other.is_directory.load(std::memory_order_relaxed),
183+
std::memory_order_relaxed);
184+
return *this;
185+
}
186+
Entry(const Entry &) = delete;
187+
auto operator=(const Entry &) -> Entry & = delete;
168188
};
169189

170190
auto track(const std::filesystem::path &path) -> void;
@@ -187,6 +207,7 @@ class SOURCEMETA_ONE_BUILD_EXPORT Build {
187207
std::vector<std::filesystem::path> &&static_dependencies,
188208
std::vector<std::filesystem::path> &&dynamic_dependencies)
189209
-> void;
210+
auto track_unsafe(const std::filesystem::path &path) -> void;
190211

191212
std::filesystem::path root;
192213
std::string root_string;

0 commit comments

Comments
 (0)