Merge pull request TravelMapping#619 from yakra/y253ll

.list processing & DB speedups

siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.cpp

@@ -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)
@@ -51,11 +44,6 @@ void HighwaySegment::add_concurrency(std::ofstream& concurrencyfile, Waypoint* w
 	other.concurrent = concurrent;
 }
 
-std::string HighwaySegment::csv_line(unsigned int id)
-{	/* return csv line to insert into a table */
-	return "'" + std::to_string(id) + "','" + std::to_string(waypoint1->point_num) + "','" + std::to_string(waypoint2->point_num) + "','" + route->root + "'";
-}
-
 std::string HighwaySegment::segment_name()
 {	/* compute a segment name based on names of all
 	concurrent routes, used for graph edge labels */

siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.h

@@ -17,15 +17,12 @@ 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();
 
 	// graph generation functions

siteupdate/cplusplus/classes/Route/Route.cpp

@@ -248,20 +248,16 @@ void Route::con_mismatch()
 			       (con_route->banner.size() ? con_route->banner : "(blank)"));
 }
 
-void Route::mark_label_in_use(char* label)
-{	usage_mtx.lock();
-	unused_alt_labels.erase(label);
-	labels_in_use.insert(label);
-	usage_mtx.unlock();
+void Route::mark_label_in_use(std::string& label)
+{	unused_alt_labels.erase(label);
+	labels_in_use.insert(move(label));
 }
 
-void Route::mark_labels_in_use(char* label1, char* label2)
-{	usage_mtx.lock();
-	unused_alt_labels.erase(label1);
+void Route::mark_labels_in_use(std::string& label1, std::string& label2)
+{	unused_alt_labels.erase(label1);
 	unused_alt_labels.erase(label2);
-	labels_in_use.insert(label1);
-	labels_in_use.insert(label2);
-	usage_mtx.unlock();
+	labels_in_use.insert(move(label1));
+	labels_in_use.insert(move(label2));
 }
 
 // sort routes by most recent update for use at end of user logs

siteupdate/cplusplus/classes/Route/Route.h

@@ -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;
@@ -93,8 +93,8 @@ class Route
 	//std::string list_line(int, int);
 	void write_nmp_merged();
 	void store_traveled_segments(TravelerList*, std::ofstream&, std::string&, unsigned int, unsigned int);
-	void mark_label_in_use(char*);
-	void mark_labels_in_use(char*, char*);
+	void mark_label_in_use(std::string&);
+	void mark_labels_in_use(std::string&, std::string&);
 	void con_mismatch();
 	size_t index();
 	Waypoint* con_beg();

siteupdate/cplusplus/classes/Route/store_traveled_segments.cpp

@@ -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())

siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp

@@ -102,27 +102,41 @@ TravelerList::TravelerList(std::string& travname, ErrorList* el)
 
 	// process lines
 	for (unsigned int l = 0; l < lines.size()-1; l++)
-	{	std::string orig_line(lines[l]);
-		// strip whitespace from beginning
-		while (lines[l][0] == ' ' || lines[l][0] == '\t') lines[l]++;
+	{	// strip whitespace from beginning
+		char *c = lines[l];
+		while (*c == ' ' || *c == '\t') c++;
+		char* beg = c;
 		// ignore whitespace or "comment" lines
-		if (lines[l][0] == 0 || lines[l][0] == '#')
-		{	splist << orig_line << endlines[l];
+		if (*c == 0 || *c == '#')
+		{	splist << lines[l] << endlines[l];
 			continue;
 		}
-		// strip whitespace from end		// first, seek to end of this line from beginning of next
-		char * endchar = lines[l+1]-2;		// -2 skips over the 0 inserted while separating listdata into lines
-		while (*endchar == 0) endchar--;	// skip back more for CRLF cases, and lines followed by blank lines
-		while (*endchar == ' ' || *endchar == '\t') *endchar-- = 0;
-		std::string trim_line(lines[l]);
-		// process fields in line
-		std::vector<char*> fields;
-		size_t spn = 0;
-		for (char* c = lines[l]; *c; c += spn)
-		{	for (spn = strcspn(c, " \t"); c[spn] == ' ' || c[spn] == '\t'; spn++) c[spn] = 0;
-			if (*c == '#') break;
-			else fields.push_back(c);
+		// split line into fields
+		std::vector<std::string> fields;
+		for (size_t spn; *c; c += spn)
+		{	if (*c == '#') break;
+			spn = strcspn(c, " \t");
+			fields.emplace_back(c,spn);
+			while (c[spn] == ' ' || c[spn] == '\t') spn++;
 		}
+		// strip whitespace from end
+		while ( strchr(" \t", fields.back().back()) ) fields.back().pop_back();
+
+		// lambda for whitespace-trimmed .list line used in userlog error reports & warnings
+		// calculate once, then it's available for re-use
+		char* trim_line = 0;
+		auto get_trim_line = [&]()
+		{	if (!trim_line)
+			{	char* end = lines[l+1]-2;	// -2 skips over 0 inserted while separating listdata into lines
+				while (*end == 0) end--;	// skip back more for CRLF cases & lines followed by blank lines
+				while (*end == ' ' || *end == '\t') end--;		  // strip whitespace @ end
+				size_t size = end-beg+1;		// +1 because end points to final good char
+				trim_line = (char*)malloc(size+1);	// +1 because need room for null terminator
+				strncpy(trim_line, beg, size);
+				trim_line[size] = 0;
+			}
+			return trim_line;
+		};
 		#define UPDATE_NOTE(R) if (R->last_update) \
 		{	updated_routes.insert(R); \
 			log << "  Route updated " << R->last_update[0] << ": " << R->readable_name() << '\n'; \
@@ -136,10 +150,11 @@ TravelerList::TravelerList(std::string& travname, ErrorList* el)
 			#include "mark_connected_route_segments.cpp"
 		     }
 		else {	log << "Incorrect format line (4 or 6 fields expected, found "
-			    << fields.size() << "): " << trim_line << '\n';
-			splist << orig_line << endlines[l];
+			    << fields.size() << "): " << get_trim_line() << '\n';
+			splist << lines[l] << endlines[l];
 		     }
 		#undef UPDATE_NOTE
+		free(trim_line);
 	}
 	delete[] listdata;
 	log << "Processed " << list_entries << " good lines marking " << clinched_segments.size() << " segments traveled.\n";

siteupdate/cplusplus/classes/TravelerList/mark_chopped_route_segments.cpp

@@ -1,5 +1,5 @@
 // find the route that matches and when we do, match labels
-std::string lookup = std::string(fields[0]) + ' ' + fields[1];
+std::string lookup = fields[0] + ' ' + fields[1];
 upper(lookup.data());
 // look for region/route combo, first in pri_list_hash
 std::unordered_map<std::string,Route*>::iterator rit = Route::pri_list_hash.find(lookup);
@@ -8,34 +8,36 @@ if (rit == Route::pri_list_hash.end())
 {	rit = Route::alt_list_hash.find(lookup);
 	if (rit == Route::alt_list_hash.end())
 	{	bool invalid_char = 0;
-		for (char& c : trim_line)
-		  if (iscntrl(c))
-		  {	c = '?';
+		for (char* c = get_trim_line(); *c; c++)
+		  if (iscntrl(*c) && *c != '\t')
+		  {	*c = '?';
 			invalid_char = 1;
 		  }
 		log << "Unknown region/highway combo in line: " << trim_line;
 		if (invalid_char) log << " [contains invalid character(s)]";
 		log << '\n';
-		splist << orig_line << endlines[l];
+		splist << lines[l] << endlines[l];
+		free(trim_line);
 		continue;
 	}
 	else	log << "Note: deprecated route name " << fields[1]
-		    << " -> canonical name " << rit->second->list_entry_name() << " in line: " << trim_line << '\n';
+		    << " -> canonical name " << rit->second->list_entry_name() << " in line: " << get_trim_line() << '\n';
 }
 Route* r = rit->second;
 if (r->system->devel())
-{	log << "Ignoring line matching highway in system in development: " << trim_line << '\n';
-	splist << orig_line << endlines[l];
+{	log << "Ignoring line matching highway in system in development: " << get_trim_line() << '\n';
+	splist << lines[l] << endlines[l];
+	free(trim_line);
 	continue;
 }
 // r is a route match, and we need to find
 // waypoint indices, ignoring case and leading
 // '+' or '*' when matching
 unsigned int index1, index2;
-while (*fields[2] == '*' || *fields[2] == '+') fields[2]++;
-while (*fields[3] == '*' || *fields[3] == '+') fields[3]++;
-upper(fields[2]);
-upper(fields[3]);
+while (fields[2][0] == '*' || fields[2][0] == '+') fields[2].assign(fields[2].data()+1);
+while (fields[3][0] == '*' || fields[3][0] == '+') fields[3].assign(fields[3].data()+1);
+upper(fields[2].data());
+upper(fields[3].data());
 // look for point indices for labels, first in pri_label_hash
 std::unordered_map<std::string,unsigned int>::iterator lit1 = r->pri_label_hash.find(fields[2]);
 std::unordered_map<std::string,unsigned int>::iterator lit2 = r->pri_label_hash.find(fields[3]);
@@ -45,13 +47,13 @@ if (lit2 == r->pri_label_hash.end()) lit2 = r->alt_label_hash.find(fields[3]);
 // if we did not find matches for both labels...
 if (lit1 == r->alt_label_hash.end() || lit2 == r->alt_label_hash.end())
 {	bool invalid_char = 0;
-	for (char& c : trim_line)
-	  if (iscntrl(c))
-	  {	c = '?';
+	for (char* c = get_trim_line(); *c; c++)
+	  if (iscntrl(*c) && *c != '\t')
+	  {	*c = '?';
 		invalid_char = 1;
 	  }
-	for (char* c = fields[2]; *c; c++) if (iscntrl(*c)) *c = '?';
-	for (char* c = fields[3]; *c; c++) if (iscntrl(*c)) *c = '?';
+	for (char& c : fields[2]) if (iscntrl(c)) c = '?';
+	for (char& c : fields[3]) if (iscntrl(c)) c = '?';
 	if (lit1 == lit2)
 		log << "Waypoint labels " << fields[2] << " and " << fields[3] << " not found in line: " << trim_line;
 	else {	log << "Waypoint label ";
@@ -60,8 +62,9 @@ if (lit1 == r->alt_label_hash.end() || lit2 == r->alt_label_hash.end())
 	     }
 	if (invalid_char) log << " [contains invalid character(s)]";
 	log << '\n';
-	splist << orig_line << endlines[l];
+	splist << lines[l] << endlines[l];
 	UPDATE_NOTE(r)
+	free(trim_line);
 	continue;
 }
 // are either of the labels used duplicates?
@@ -75,24 +78,24 @@ if (r->duplicate_labels.count(fields[3]))
 	duplicate = 1;
 }
 if (duplicate)
-{	splist << orig_line << endlines[l];
+{	splist << lines[l] << endlines[l];
 	log << "  Please report this error in the Travel Mapping forum.\n  Unable to parse line: "
-	    << trim_line << '\n';
+	    << get_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();
+	free(trim_line);
 	continue;
 }
 // if both labels reference the same waypoint...
 if (lit1->second == lit2->second)
-{	log << "Equivalent waypoint labels mark zero distance traveled in line: " << trim_line << '\n';
-	splist << orig_line << endlines[l];
+{	log << "Equivalent waypoint labels mark zero distance traveled in line: " << get_trim_line() << '\n';
+	splist << lines[l] << endlines[l];
 	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;
@@ -102,7 +105,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)
 	{
@@ -112,5 +120,5 @@ else {	r->system->mark_route_in_use(lookup);
 		#undef r1
 		#undef r2
 	}
-	else	splist << orig_line << endlines[l];
+	else	splist << lines[l] << endlines[l];
      }