Skip to content

Commit

Permalink
ReadListThread mutex efficiency
Browse files Browse the repository at this point in the history
Squash 85b2a32, 794a0be, 90f08a1
* Nix HighwaySegment::clin_mtx; use Route::mtx
* Lock/unlock once per store_traveled_segments call
* Consolidate lock/unlock ops w/calls to mark_label(s)_in_use
  • Loading branch information
yakra committed Mar 29, 2024
1 parent 9f95b0d commit 44d62f0
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ std::string HighwaySegment::str()
{ return route->readable_name() + " " + waypoint1->label + " " + waypoint2->label;
}

bool HighwaySegment::add_clinched_by(size_t t)
{ clin_mtx.lock();
bool result = clinched_by.add_index(t);
clin_mtx.unlock();
return result;
}

void HighwaySegment::add_concurrency(std::ofstream& concurrencyfile, Waypoint* w)
{ HighwaySegment& other = w->route->segments[w - w->route->points.data];
if (!concurrent)
Expand Down
2 changes: 0 additions & 2 deletions siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ class HighwaySegment
double length;
std::list<HighwaySegment*> *concurrent;
TMBitset<TravelerList*, uint32_t> clinched_by;
std::mutex clin_mtx;

HighwaySegment(Waypoint*, Route*);
~HighwaySegment();

std::string str();
bool add_clinched_by(size_t);
void add_concurrency(std::ofstream&, Waypoint*);
std::string csv_line(unsigned int);
//std::string concurrent_travelers_sanity_check();
Expand Down
8 changes: 2 additions & 6 deletions siteupdate/cplusplus/classes/Route/Route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,15 @@ void Route::con_mismatch()
}

void Route::mark_label_in_use(char* label)
{ usage_mtx.lock();
unused_alt_labels.erase(label);
{ unused_alt_labels.erase(label);
labels_in_use.insert(label);
usage_mtx.unlock();
}

void Route::mark_labels_in_use(char* label1, char* label2)
{ usage_mtx.lock();
unused_alt_labels.erase(label1);
{ unused_alt_labels.erase(label1);
unused_alt_labels.erase(label2);
labels_in_use.insert(label1);
labels_in_use.insert(label2);
usage_mtx.unlock();
}

// sort routes by most recent update for use at end of user logs
Expand Down
2 changes: 1 addition & 1 deletion siteupdate/cplusplus/classes/Route/Route.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Route
std::unordered_set<std::string> unused_alt_labels;
std::unordered_set<std::string> duplicate_labels;
std::unordered_map<std::string, unsigned int> pri_label_hash, alt_label_hash;
std::mutex usage_mtx; // for locking labels_in_use & unused_alt_labels during TravelerList processing
std::mutex mtx;
TMArray<HighwaySegment> segments;
std::string* last_update;
double mileage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
#include "../TravelerList/TravelerList.h"
#include <fstream>

void Route::store_traveled_segments(TravelerList* t, std::ofstream& log, std::string& update, unsigned int beg, unsigned int end)
void Route::store_traveled_segments(TravelerList* t, std::ofstream& log, std::string& update, unsigned int beg, unsigned int endex)
{ // store clinched segments with traveler and traveler with segments
size_t index = t-TravelerList::allusers.data;
for (unsigned int pos = beg; pos < end; pos++)
{ HighwaySegment *hs = segments.data+pos;
if (hs->add_clinched_by(index))
for (HighwaySegment *hs = segments.data+beg, *end = segments.data+endex; hs < end; hs++)
if (hs->clinched_by.add_index(index))
t->clinched_segments.push_back(hs);
}
#ifdef threading_enabled
// create key/value pairs in regional tables, to be computed in a threadsafe manner later
if (system->active())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ if (duplicate)
log << " Please report this error in the Travel Mapping forum.\n Unable to parse line: "
<< trim_line << '\n';
r->system->mark_route_in_use(lookup);
r->mtx.lock();
r->mark_labels_in_use(fields[2], fields[3]);
r->mtx.unlock();
continue;
}
// if both labels reference the same waypoint...
Expand All @@ -89,10 +91,7 @@ if (lit1->second == lit2->second)
UPDATE_NOTE(r)
}
// otherwise both labels are valid; mark in use & proceed
else { r->system->mark_route_in_use(lookup);
r->mark_labels_in_use(fields[2], fields[3]);

list_entries++;
else { list_entries++;
bool reverse = 0;
if (lit1->second <= lit2->second)
{ index1 = lit1->second;
Expand All @@ -102,7 +101,12 @@ else { r->system->mark_route_in_use(lookup);
index2 = lit1->second;
reverse = 1;
}
r->mtx.lock();
r->store_traveled_segments(this, log, update, index1, index2);
r->mark_labels_in_use(fields[2], fields[3]);
r->mtx.unlock();
r->system->mark_route_in_use(lookup);

// new .list lines for region split-ups
if (Args::splitregion == r->region->code)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ if (duplicate)
log << " Please report this error in the Travel Mapping forum.\n"
<< " Unable to parse line: " << trim_line << '\n';
r1->system->mark_routes_in_use(lookup1, lookup2);
r1->mark_label_in_use(fields[2]);
r2->mark_label_in_use(fields[5]);
r1->mtx.lock(); r1->mark_label_in_use(fields[2]); r1->mtx.unlock();
r2->mtx.lock(); r2->mark_label_in_use(fields[5]); r2->mtx.unlock();
continue;
}
bool reverse = 0;
Expand All @@ -121,10 +121,12 @@ if (r1 == r2)
UPDATE_NOTE(r1)
continue;
}
r1->mtx.lock();
if (index1 <= index2)
r1->store_traveled_segments(this, log, update, index1, index2);
else r1->store_traveled_segments(this, log, update, index2, index1);
r1->mark_labels_in_use(fields[2], fields[5]);
r1->mtx.unlock();
}
else { // user log warning for DISCONNECTED_ROUTE errors
if (r1->con_route->disconnected)
Expand All @@ -144,23 +146,31 @@ else { // user log warning for DISCONNECTED_ROUTE errors
index1 = lit2->second;
index2 = lit1->second;
reverse = 1;
r1->mark_label_in_use(fields[5]);
r2->mark_label_in_use(fields[2]);
}
else { r1->mark_label_in_use(fields[2]);
r2->mark_label_in_use(fields[5]);
}

// mark the beginning chopped route from index1 to its end
r1->mtx.lock();
r1->mark_label_in_use(reverse ? fields[5] : fields[2]);
if (r1->is_reversed())
r1->store_traveled_segments(this, log, update, 0, index1);
else r1->store_traveled_segments(this, log, update, index1, r1->segments.size);
r1->mtx.unlock();

// mark the ending chopped route from its beginning to index2
r2->mtx.lock();
r2->mark_label_in_use(reverse ? fields[2] : fields[5]);
if (r2->is_reversed())
r2->store_traveled_segments(this, log, update, index2, r2->segments.size);
else r2->store_traveled_segments(this, log, update, 0, index2);
r2->mtx.unlock();

// mark any intermediate chopped routes in their entirety.
for (size_t r = r1->rootOrder+1; r < r2->rootOrder; r++)
r1->con_route->roots[r]->store_traveled_segments(this, log, update, 0, r1->con_route->roots[r]->segments.size);
{ auto& cr = r1->con_route->roots[r];
cr->mtx.lock();
cr->store_traveled_segments(this, log, update, 0, cr->segments.size);
cr->mtx.unlock();
}
}
r1->system->mark_routes_in_use(lookup1, lookup2);
list_entries++;
Expand Down
2 changes: 1 addition & 1 deletion siteupdate/cplusplus/tasks/threaded/ConcAug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
for (HighwaySegment *s : t->clinched_segments)
if (s->concurrent)
for (HighwaySegment *hs : *(s->concurrent))
if (hs != s && hs->route->system->active_or_preview() && hs->add_clinched_by(index))
if (hs != s && hs->route->system->active_or_preview() && hs->clinched_by.add_index(index))
concurrencyfile << "Concurrency augment for traveler " << t->traveler_name << ": [" << hs->str() << "] based on [" << s->str() << "]\n";
}
cout << '!' << endl;
Expand Down
19 changes: 12 additions & 7 deletions siteupdate/cplusplus/threads/ConcAugThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ void ConcAugThread(unsigned int id, std::mutex* mtx, std::vector<std::string>* a
for (HighwaySegment *s : t->clinched_segments)
if (s->concurrent)
for (HighwaySegment *hs : *(s->concurrent))
if (hs != s && hs->route->system->active_or_preview() && hs->add_clinched_by(index))
{ augment_list->push_back("Concurrency augment for traveler " + t->traveler_name + ": [" + hs->str() + "] based on [" + s->str() + ']');
// create key/value pairs in regional tables, to be computed in a threadsafe manner later
if (hs->route->system->active())
t->active_only_mileage_by_region[hs->route->region];
t->active_preview_mileage_by_region[hs->route->region];
t->system_region_mileages[hs->route->system][hs->route->region];
if (hs != s && hs->route->system->active_or_preview())
{ hs->route->mtx.lock();
bool inserted = hs->clinched_by.add_index(index);
hs->route->mtx.unlock();
if (inserted)
{ augment_list->push_back("Concurrency augment for traveler " + t->traveler_name + ": [" + hs->str() + "] based on [" + s->str() + ']');
// create key/value pairs in regional tables, to be computed in a threadsafe manner later
if (hs->route->system->active())
t->active_only_mileage_by_region[hs->route->region];
t->active_preview_mileage_by_region[hs->route->region];
t->system_region_mileages[hs->route->system][hs->route->region];
}
}
}
}

0 comments on commit 44d62f0

Please sign in to comment.