Previous iterator should only be set when the node is alive.

Summary: There was a big performance hit induced when removing consecutive nodes from the concurrent list. The reason why it was happening is that the iterator has a pointer to previous node, and uses that pointer to re-link the whole list after the deletion. That node wasn't alive because it was deleted earlier and was always being updated to the next deleted entry in the list while incrementing the iterator. This behaviour caused find_and_disconnect method to be invoked, which has an O(n) complexity. That made our removal of O(n) entries from the list run in O(n^2) time, which is obviously slow.

Reviewers: buda, mislav.bradac, florijan

Reviewed By: mislav.bradac

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D483
This commit is contained in:
Dominik Gleich 2017-06-16 13:37:12 +02:00
parent 9ed4102897
commit f034c29724

View File

@ -127,7 +127,11 @@ class ConcurrentList {
It &operator++() {
debug_assert(valid(), "Not valid data.");
do {
prev = curr;
// We don't care about previous unless it's alive. If it's not alive we
// are going to look for it again and just incurr performance hit
// because of constant lookup of previous alive iterator while
// re-linking.
if (!curr->removed) prev = curr;
curr = load(curr->next);
} while (valid() && is_removed()); // Loop ends if end of list is
// found or if not removed