making better use of std::unordered map

[yakra]

Looking at DBFieldLength.h for #119, I starting looking into when to use constexpr, which led me here, then here.

My personal recommendation is to avoid the [] operator. If you want to use an entry if it exists, then use map::find().

auto item = m.find(2);
if (item != m.end()) {
 DoSomethingWith(*item);
}

Reminded me that a lot of the time I compare the result of std::unordered_map::find to std::unordered_map::end ignore it when I could be saving it to a variable, and then waste time doing a lookup again later, usually with std::unordered_map::at.

That article led me to this video. Neato. I'd noticed the behavior the presenter describes, but avoided it due to the same "newbie" expectations he cites, of uninitialized rather than value-initialized doubles. Thought it was possible that not every C++ compiler would gimme the results I was seeing. Great news. This could mean good things for compute_stats_t. Finally, the first of these two StackOverflow threads leads right back to Louis Brandy's video.

There are several ways to better use std::unordered map:

catch

Consider using std::unordered_map::find, comparing to std::unordered_map::end and dereferencing the result instead of (potentially expensively?) constructing std::out_of_range objects.

• [ ] allbyregionactiveonly
• [ ] allbyregionactivepreview
• [ ] tasks/read_updates.cpp
• [ ] HighwaySystem::stats_csv
• [x] ConnectedRoute::ConnectedRoute should stay as-is, because ErrorList.
• [x] Route::Route should stay as-is, because ErrorList.
• [ ] Route::compute_stats_r: [] +=
• [x] HighwaySegment::compute_stats_t: [] +=
• [x] TravelerList::TravelerList (as of d56e5a6) can stay. A populated listupdates set with missing or malformed entries, missing info from git, should happen just about never in production. If it does, one lookup per traveler is inexpensive.

count

Consider using std::unordered_map::find and dereferencing the result instead of discarding it and searching again with std::unordered_map::at or whatever All there is right now, at least in the master branch, is 9e2e4ef.

• [ ] Save result of active_only_mileage_by_region.find(region) as an iterator
• [ ] review how system_region_miles works (and all occurrences) -- does/can it just return 0?
• [ ] first half unnecessary

all occurrences

[] may be accompanied by other unnecessary operations .find cases can often have results saved in a variable insert and emplace may benefit from better use of their respective construction methods; try piecewise-construct ...and whatever else

• [ ] active_only_mileage_by_region
• [ ] active_preview_mileage_by_region
• [ ] alt_label_hash
• [ ] alt_list_hash
• [ ] code_hash
• [ ] listupdates
• [ ] mileage_by_region (and sysmbr in the r2 branch)
• [ ] pri_label_hash
• [ ] pri_list_hash
• [ ] root_hash
• [ ] system_region_mileages

[yakra]

Interesting -- https://www.youtube.com/watch?v=M2fKMP47slQ suggests that for small containers, std::map is faster than std::unordered_map. See also https://stackoverflow.com/questions/2196995/is-there-any-advantage-of-using-map-over-unordered-map-in-case-of-trivial-keys/39864767#39864767

• [ ] HighwaySystem::mileage_by_region is most likely within this range.
• [ ] Cache misses: Linear search faster for smaller containers, std::map slower
• [ ] Look for others.

[yakra]

^ TravelerList: unordered_map -> map CompStatsT: faster with -O3 but not -O0.

• [ ] Test in isolation from any CompStatsR changes.

Variables & their affected tasks:

• std::map<Region*, double> active_preview_mileage_by_region;
  • allbyregionactivepreview
  • "...clinchedOverallMileageByRegion"
  • CompStatsT
  • UserLog
• std::map<Region*, double> active_only_mileage_by_region;
  • allbyregionactiveonly
  • "...clinchedOverallMileageByRegion"
  • CompStatsT
  • UserLog
• std::map<HighwaySystem*, std::map<Region*, double>> system_region_mileages;
  • "...clinchedSystemMileageByRegion"
  • 'per-system stats csv files'
  • CompStatsT
  • UserLog