detecting duplicate labels during .list processing

[yakra]

When processing .list lines, each label is looked up in at least 2 hash tables:

1. Route::duplicate_labels
2. Route::pri_label_hash
3. Route::alt_list_hash, if not found in pri_label_hash

Instead:

• Nix Route::duplicate_labels. This will also save a good chunk of RAM added up over all Routes.
• When we detect a duplicate creating label hashes, set the label's value in Route::pri_label_hash to -1 (AKA 4294967295).
• When parsing .list lines, as soon as we get our iterators back from looking up both labels, check for an index of -1.
  Nope, gotta do it the same place we already do, to ensure both iterators are valid (not end()).
• Then do what we do now -- userlog notification and continue;
• A lambda can cut down on boilerplate. We have the same block of code twice each in both the 4-field & 6-field routines.

Since a good chunk of mark_connected_route_segments.cpp will get shuffled around to enable 6-field support for hidden systems, it makes sense to do this now.

[yakra]

Well. That was underwhelming.

The greatest time savings I saw was 0.126 s on bsdlab @ 1 thread. And I think that may have been an outlier.
At more typical thread counts like 4+, time savings topped out at about 16 ms.
Just in case this task is I/O-bound, I tried it again using a RAM disk. Same results.

RAM savings? 5.65 MB for the containers, 3392 B for the labels themselves, + whatever "under the hood" cost.
That's on Ubuntu. Both take up less space on FreeBSD.

On the positive side, I guess std::hash<std::string> is pretty darn efficient!

---

I almost don't wanna push this...
Sometimes when debugging I'll go to wherever in main() and start doing

Route* r = Route::root_hash.at("me.i295");
unsigned int index = r->pri_label_hash.at("6");
Waypoint& w = r->points[index];
HighwaySegment& s = r->segments[index];
// crunch some data & print stuff out...

If I do that with an index of -1 I get an invalid reference.
OTOH, doing that with duplicate labels may not get me the point I want anyway, and I'd be better off hard-coding the line number (-1) from the .wpt file.

Any thoughts on this?

[michihdeu]

Any thoughts on this?

Region split first, please 🙏

[yakra]

FWIW this is already written and tested.
Made sense to try it out before making other changes to mark_connected_route_segments.cpp and then trying to fit this in afterward. Easier to avoid potential merge conflicts.

Either way, I've set this aside and am back to working on the region split.
Right now, while waiting to see if there are any replies to this post about datachecks -- O HAY, there's one! -- I've been hacking away at 6-field support.

So far, I've moved this block of code down here between the { and the //,
and then spliced in this monstrosity after the UPDATE_NOTEs:

std::vector<ConnectedRoute*> candidates;
int prev_rank = 0;
Waypoint::iterator wit1(r1->points[index1]);
if ((*wit1)->route->system->level == 'd') wit1.next_live();
do {	ConnectedRoute* cr1 = (*wit1)->route->con_route;
	Waypoint::iterator wit2(r2->points[index2]);
	if ((*wit2)->route->system->level == 'd') wit2.next_live();
	do {	ConnectedRoute* cr2 = (*wit2)->route->con_route;
		if (cr1 == cr2)
		{	int this_rank = 2*(cr2 == r2->con_route || cr1 == r1->con_route);
			if (this_rank == 0)
			{   this_rank =
				cr2->route == r2->route && cr2->banner == r2->banner
			     || cr1->route == r1->route && cr1->banner == r1->banner
			     || cr1->route == r2->route && cr1->banner == r2->banner
			     || cr2->route == r1->route && cr2->banner == r1->banner;
			}
			if (prev_rank > this_rank)
			    continue;
			if (prev_rank < this_rank)
			{   prev_rank = this_rank;
			    candidates.assign({cr1});
			}
			else if (!contains(candidates, cr1))
			    candidates.push_back(cr1);
		}
		wit2.next_live();
	   } while (*wit2);
	wit1.next_live();
   } while (*wit1);

...and that's where I left off about 6 hours ago.
The stuff in the inner do-while loop is actually a horrible idea; as for why, I'll leave that as an exercise for the reader. ;)
I'm coming to think that this stuff might be better tested after making sure Italy is set to go and starting the switchover process. Any additional work on datachecks can probably wait till after hidden systems go live.
I'll take the rest of this discussion over to the forum.