-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
concurrency detection memory leak #449
Comments
colocation lists
|
for (HighwaySystem* h : HighwaySystem::syslist) // h = eure
{cout << '.' << flush; //
for (Route* r : h->route_list) // r = pol.e67
for (HighwaySegment* s : r->segment_list) // s = [POL E67 29(S8) 27(A1)]
if (s->waypoint1->colocated && s->waypoint2->colocated) // both colocated; proceed
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.a001@26
if (w1->route != r) // diff route; proceed
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.a001@27
if (w1->route == w2->route) // same route; proceed
{HighwaySegment* other = w1->route->find_segment_by_waypoints(w1,w2); // other = [POL A1 26 27]
if (other) // proceed
if (!s->concurrent) // First pass; s->concurrent still 0; proceed
{s->concurrent = new list<HighwaySegment*>; // new list
other->concurrent = s->concurrent; // 2 pointers to the same list
s->concurrent->push_back(s); // E67 S & A1 get E67 S
s->concurrent->push_back(other); // E67 S & A1 get A1
//concurrencyfile << "New concurrency [" etc. //
} //
else //...
}
|
// w1 = pol.a001@26
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.dk074@A1_S, etc.
if (w1->route == w2->route) // False for all remaining
|
// s = [POL E67 29(S8) 27(A1)]
if (s->waypoint1->colocated && s->waypoint2->colocated) // both colocated; proceed
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.dk074@A1_N
if (w1->route != r) // diff route; proceed
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.dk074@A1_S
if (w1->route == w2->route) // same route; proceed
{HighwaySegment* other = w1->route->find_segment_by_waypoints(w1,w2); // other = [POL DK74 A1_S A1_N]
if (other) // proceed
if (!s->concurrent) // assigned a value in prev pass; skip
//... //
else if (!contains(*s->concurrent, other)) // not in list; proceed
{other->concurrent = s->concurrent; // add list to DK74
s->concurrent->push_back(other); // add DK74 to list
//concurrencyfile << "Extended concurrency " etc. //
}
}
|
// s = [POL E67 29(S8) 27(A1)]
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.e67@26(A1), pol.e67@29(S8)
if (w1->route != r) // same route; skip
OK, I think I see where this is going... |
We continue on with
We're done checking Why is PA different?The first route processed is I-376, not the self-concurrent one. All other routes pass the |
for (HighwaySegment* s : r->segment_list) // s = [POL E67 27(A1) 26(A1)]
if (s->waypoint1->colocated && s->waypoint2->colocated) // both colocated; proceed
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.a001@27
if (w1->route != r) // diff route; proceed
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.a001@26
if (w1->route == w2->route) // same route; proceed
{HighwaySegment* other = w1->route->find_segment_by_waypoints(w1,w2); // other = [POL A1 26 27]
if (other) // proceed
if (!s->concurrent) // s->concurrent still 0; proceed
{s->concurrent = new list<HighwaySegment*>; // a 2nd list is created
other->concurrent = s->concurrent; // [POL A1 26 27]'s list overwritten
s->concurrent->push_back(s); // E67 N & A1 get E67 N
s->concurrent->push_back(other); // E67 N & A1 get A1
//concurrencyfile << "New concurrency [" etc. //
} //
else //... //
}
|
// s = [POL E67 27(A1) 26(A1)]
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.dk074@A1_S
if (w1->route != r) // diff route; proceed
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.dk074@A1_N
if (w1->route == w2->route) // same route; proceed
{HighwaySegment* other = w1->route->find_segment_by_waypoints(w1,w2); // other = [POL DK74 A1_S A1_N]
if (other) // proceed
if (!s->concurrent) // list exists since we found A1; skip
//... //
else if (!contains(*s->concurrent, other)) // not in list; proceed
{other->concurrent = s->concurrent; // [POL DK74 A1_S A1_N]'s list overwritten
s->concurrent->push_back(other); // add DK74 to list
//concurrencyfile << "Extended concurrency " etc. //
}
}
|
// s = [POL E67 27(A1) 26(A1)]
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.e67@27(A1)
if (w1->route != r) // same route; skip We continue on with
At this point the 7 concurrent segments share 2 concurrency lists, each listing 6 segments:
We're done checking |
The A1 & DK74 segments are already contained in for (Route* r : h->route_list) // r = pol.e75
for (HighwaySegment* s : r->segment_list) // s = [POL E75 26(A1) 27(A1)]
if (s->waypoint1->colocated && s->waypoint2->colocated) // both colocated; proceed
for ( Waypoint* w1 : *(s->waypoint1->colocated) ) // w1 = pol.e67@26(A1)
if (w1->route != r) // diff route; proceed
for ( Waypoint* w2 : *(s->waypoint2->colocated) ) // w2 = pol.e67@27(A1)
if (w1->route == w2->route) // same route; proceed
{HighwaySegment* other = w1->route->find_segment_by_waypoints(w1,w2); // other = [POL E67 27(A1) 26(A1)]
if (other) // proceed
if (!s->concurrent) // list exists; skip
//... //
else if (!contains(*s->concurrent, other)) // not in the new list yet; proceed
{other->concurrent = s->concurrent; // [POL E67 27(A1) 26(A1)]'s list overwritten
s->concurrent->push_back(other); // add E67 S to list
//concurrencyfile << "Extended concurrency " etc. //
}
} And there's the memory leak. The last pointer to the original concurrency list is overwritten, making it inaccessible.
All 7 segments point to the new list, which has all 7 segments on it. The proposal this time is a bit different from that in #137; code should be leaner & cleaner, smaller diff overall, and should probably run a tiny bit faster. |
Graph Generation
DataProcessing/siteupdate/python-teresco/siteupdate.py Lines 1812 to 1819 in 3baef90
The graph generation process has changed since this check was first implemented. It's not as necessary as it once was. This check is still necessary for cases like UT190 when a route is concurrent with itself but no other routes. |
People who've marked both segments will see:
Based8.log:
3,4c3,4
< Overall in active systems: 61034.46 of 1145620.61 mi (5.33%)
< Overall in active+preview systems: 62409.09 of 1548324.46 mi (4.03%)
---
> Overall in active systems: 61034.13 of 1145620.28 mi (5.33%)
> Overall in active+preview systems: 62408.76 of 1548324.13 mi (4.03%)
45c45
< UT: 1080.46 of 6025.71 mi (17.93%), 1080.46 of 6025.71 mi (17.93%)
---
> UT: 1080.12 of 6025.38 mi (17.93%), 1080.12 of 6025.38 mi (17.93%)
3735c3735
< System usaut (active) overall: 303.15 of 3704.01 mi (8.18%)
---
> System usaut (active) overall: 302.82 of 3703.68 mi (8.18%) People who've marked only 1 segment will see:
bobcobb.log:
18,19c18,19
< Overall in active systems: 68285.42 of 1145620.61 mi (5.96%)
< Overall in active+preview systems: 70729.67 of 1548324.46 mi (4.57%)
---
> Overall in active systems: 68285.42 of 1145620.28 mi (5.96%)
> Overall in active+preview systems: 70729.67 of 1548324.13 mi (4.57%)
66c66
< UT: 2084.86 of 6025.71 mi (34.60%), 2084.86 of 6025.71 mi (34.60%)
---
> UT: 2084.86 of 6025.38 mi (34.60%), 2084.86 of 6025.38 mi (34.60%)
3536c3536
< System usaut (active) overall: 378.56 of 3704.01 mi (10.22%)
---
> System usaut (active) overall: 378.56 of 3703.68 mi (10.22%)
3598c3598
< UT190: 16.44 of 19.80 mi (83.03%)
---
> UT190: 16.77 of 19.80 mi (84.72%) |
penultimate rebase on BiggaTomato: |
* speedup; find all concurrencies on 1st pass; ignore segments on subsequent passes * prevent memory leak from overwritten/orphaned concurrency lists * "reverse" check in HGEdge ctor no longer needed interactive rebase d54bfbd89ced77e04655b9ac5a971b01551c59a5
Valgrind reports:
An orphaned concurrency list is not attached to any HighwaySegment. It's created, partially populated, and then its pointer is overwritten during concurrency detection.
std::_List_node
s)POL E67 29(S8) 27(A1)
The first list constructed gets built out to 6 of 7 routes, then its pointer is overwritten.POL E67 27(A1) 26(A1)
POL E75 26(A1) 27(A1)
POL A1 26 27
POL S8 26(A1) 27(A1)
POL S8 27(A1) 29
POL DK74 A1_S A1_N
The text was updated successfully, but these errors were encountered: