diff --git a/lib/ts/PriorityQueue.h b/lib/ts/PriorityQueue.h index 8e27cb31f0d..06ea052ea94 100644 --- a/lib/ts/PriorityQueue.h +++ b/lib/ts/PriorityQueue.h @@ -48,6 +48,7 @@ template > class PriorityQueue PriorityQueue() {} ~PriorityQueue() {} bool empty(); + bool in(PriorityQueueEntry *entry); PriorityQueueEntry *top(); void pop(); void push(PriorityQueueEntry *); @@ -71,6 +72,13 @@ PriorityQueue::dump() const return _v; } +template +bool +PriorityQueue::in(PriorityQueueEntry *entry) +{ + return _v.in(entry) != NULL; +} + template bool PriorityQueue::empty() diff --git a/proxy/http2/Http2DependencyTree.h b/proxy/http2/Http2DependencyTree.h index 8ad7fb01b2d..70aed802103 100644 --- a/proxy/http2/Http2DependencyTree.h +++ b/proxy/http2/Http2DependencyTree.h @@ -250,6 +250,8 @@ Http2DependencyTree::reprioritize(Node *node, uint32_t new_parent_id, bool ex // Do nothing return; } + // should not change the root node + ink_assert(node->parent); Node *new_parent = find(new_parent_id); if (new_parent == NULL) { @@ -264,11 +266,29 @@ template void Http2DependencyTree::_change_parent(Node *node, Node *new_parent, bool exclusive) { + ink_release_assert(node->parent != NULL); node->parent->children.remove(node); + if (node->queued) { + node->parent->queue->erase(node->entry); + node->queued = false; + + Node *current = node->parent; + while (current->queue->empty() && !current->active && current->parent != NULL) { + current->parent->queue->erase(current->entry); + current->queued = false; + current = current->parent; + } + } + node->parent = NULL; if (exclusive) { while (Node *child = new_parent->children.pop()) { + if (child->queued) { + child->parent->queue->erase(child->entry); + node->queue->push(child->entry); + } + node->children.push(child); child->parent = node; } @@ -276,6 +296,15 @@ Http2DependencyTree::_change_parent(Node *node, Node *new_parent, bool exclus new_parent->children.push(node); node->parent = new_parent; + + if (node->active || !node->queue->empty()) { + Node *current = node; + while (current->parent != NULL && !current->queued) { + current->parent->queue->push(current->entry); + current->queued = true; + current = current->parent; + } + } } template diff --git a/proxy/http2/test_Http2DependencyTree.cc b/proxy/http2/test_Http2DependencyTree.cc index a1476850c98..896cc89cb52 100644 --- a/proxy/http2/test_Http2DependencyTree.cc +++ b/proxy/http2/test_Http2DependencyTree.cc @@ -530,6 +530,132 @@ REGRESSION_TEST(Http2DependencyTree_exclusive_node)(RegressionTest *t, int /* at delete tree; } +/** test for reprioritize with active node +* +* root root root +* / \ / \ (remove A) / \ +* A B =======> C B =======> C B +* \ / +* C A +* +*/ +REGRESSION_TEST(Http2DependencyTree_reprioritize)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus) +{ + TestBox box(t, pstatus); + box = REGRESSION_TEST_PASSED; + + Tree *tree = new Tree(100); + string a("A"), b("B"), c("C"); + + Tree::Node *A = tree->add(0, 7, 70, false, &a); + Tree::Node *B = tree->add(0, 3, 10, false, &b); + Tree::Node *C = tree->add(3, 5, 30, false, &c); + + tree->activate(A); + tree->activate(B); + tree->activate(C); + + tree->reprioritize(A, 5, false); + + tree->deactivate(A, 0); + tree->remove(A); + + box.check(tree->top()->t != nullptr, "should not core dump"); + + delete tree; +} + +/** + * Reprioritization (exclusive) + * + * x x + * | | + * A D + * / \ | + * B C ==> A + * / \ /|\ + * D E B C F + * | | + * F E + */ +REGRESSION_TEST(Http2DependencyTree_reprioritize_2)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus) +{ + TestBox box(t, pstatus); + box = REGRESSION_TEST_PASSED; + + Tree *tree = new Tree(100); + string a("A"), b("B"), c("C"), d("D"), e("E"), f("F"); + + tree->add(0, 1, 0, false, &a); + tree->add(1, 3, 0, false, &b); + tree->add(1, 5, 0, false, &c); + tree->add(5, 7, 0, false, &d); + tree->add(5, 9, 0, false, &e); + tree->add(7, 11, 0, false, &f); + + Tree::Node *node_x = tree->find(0); + Tree::Node *node_a = tree->find(1); + Tree::Node *node_b = tree->find(3); + Tree::Node *node_d = tree->find(7); + + tree->activate(node_b); + box.check(node_x->queue->in(node_a->entry), "A should be in x's queue"); + + tree->reprioritize(1, 7, true); + + box.check(!node_x->queue->in(node_a->entry), "A should not be in x's queue"); + box.check(node_x->queue->in(node_d->entry), "D should be in x's queue"); + box.check(node_d->queue->in(node_a->entry), "A should be in d's queue"); + + delete tree; +} + +/** + * Reprioritization (exclusive) + * + * x x + * | | + * A D + * / \ | + * B C ==> A + * / \ /|\ + * D E B C F + * | | + * F E + */ +REGRESSION_TEST(Http2DependencyTree_reprioritize_3)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus) +{ + TestBox box(t, pstatus); + box = REGRESSION_TEST_PASSED; + + Tree *tree = new Tree(100); + string a("A"), b("B"), c("C"), d("D"), e("E"), f("F"); + + tree->add(0, 1, 0, false, &a); + tree->add(1, 3, 0, false, &b); + tree->add(1, 5, 0, false, &c); + tree->add(5, 7, 0, false, &d); + tree->add(5, 9, 0, false, &e); + tree->add(7, 11, 0, false, &f); + + Tree::Node *node_x = tree->find(0); + Tree::Node *node_a = tree->find(1); + Tree::Node *node_c = tree->find(5); + Tree::Node *node_d = tree->find(7); + Tree::Node *node_f = tree->find(11); + + tree->activate(node_f); + tree->reprioritize(1, 7, true); + + box.check(node_a->queue->in(node_f->entry), "F should be in A's queue"); + box.check(node_d->queue->in(node_a->entry), "A should be in D's queue"); + box.check(node_x->queue->in(node_d->entry), "D should be in x's queue"); + box.check(!node_a->queue->in(node_c->entry), "C should not be in A's queue"); + box.check(node_c->queue->empty(), "C's queue should be empty"); + + delete tree; +} + REGRESSION_TEST(Http2DependencyTree_max_depth)(RegressionTest *t, int /* atype ATS_UNUSED */, int *pstatus) { TestBox box(t, pstatus);