Skip to content

Unique names cleanup - #190

Merged
jteresco merged 6 commits into
TravelMapping:masterfrom
yakra:unique_names
Mar 12, 2019
Merged

Unique names cleanup#190
jteresco merged 6 commits into
TravelMapping:masterfrom
yakra:unique_names

Conversation

@yakra

@yakrayakra commented Feb 7, 2019

Copy link
Copy Markdown
Contributor

This branch also contains the commits in #188.

Right now, vertex labels are stored in four places:

  • as keys of unique_waypoints dict
  • within waypoints
  • within vertices themselves
  • as keys of vertices dict

With 808,995 points at 551,932 unique locations, the memory usage can add up.
With a slightly older revision of the HighwayData repo, I calculated potential savings of 24,703,931 B for just the strings themselves.
Add in the overhead of the std::string container itself, and it goes up to 86,716,079 B savings in C++.
(I don't know what the string container overhead works out to in Python.)

This cuts down on the duplication:

  • The unique_waypoints dict is replaced with a set, to track the unique names as they're generated.
    • Unique waypoint locations themselves are determined by whether a waypoint is a singleton waypoint, or at the front of its colocation list.
    • This set is deleted when vertex construction is complete and it's no longer needed.
  • No more storage within waypoints.
    • The HighwayGraph constructor instead passes point_name directly to the vertex constructor, cutting out the Waypoint object middleman.
  • The vertices themselves are now the only place permanently storing unique names.
  • The vertices dict uses canonical waypoint objects as keys, rather than unique names.

This also results in slight time savings during graph setup, about 3.8s on BiggaTomato.

@yakra

Copy link
Copy Markdown
ContributorAuthor

Putting this on hold until I get yakra#64 sorted.

@yakrayakra reopened this Feb 10, 2019
@yakrayakra mentioned this pull request Feb 11, 2019
@jteresco
jteresco merged commit a333498 into TravelMapping:masterMar 12, 2019
@yakra
yakra deleted the unique_names branch March 16, 2019 18:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@yakra@jteresco