Repository: wicket Updated Branches: refs/heads/WICKET-6506 [created] 0dc04e448
WICKET-6506: Fix performance regression Due to the way modcount == 0 was handled, the iterator was reinitialized on every call to next. The fix is to no longer reset the modcount on detach but increment it. This even fixed a corner case that was marked in the tests. Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/0dc04e44 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/0dc04e44 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/0dc04e44 Branch: refs/heads/WICKET-6506 Commit: 0dc04e448ba31de15d8c32b1ee964bebed55bc36 Parents: e7597f0 Author: Emond Papegaaij <[email protected]> Authored: Thu Dec 14 17:03:35 2017 +0100 Committer: Emond Papegaaij <[email protected]> Committed: Thu Dec 14 17:03:35 2017 +0100 ---------------------------------------------------------------------- .../java/org/apache/wicket/MarkupContainer.java | 4 ++-- .../org/apache/wicket/MarkupContainerTest.java | 22 +++----------------- 2 files changed, 5 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/0dc04e44/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java index a9ffd82..36b7684 100644 --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java @@ -589,7 +589,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp private void refreshInternalIteratorIfNeeded() { - if (modCounter != 0 && expectedModCounter >= modCounter) + if (expectedModCounter >= modCounter) return; if (children == null) @@ -1808,7 +1808,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp { super.onDetach(); - modCounter = 0; + modCounter++; removals_clear(); if (queue != null && !queue.isEmpty() && hasBeenRendered()) http://git-wip-us.apache.org/repos/asf/wicket/blob/0dc04e44/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java b/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java index edf3f7f..4ab69da 100644 --- a/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/MarkupContainerTest.java @@ -1124,14 +1124,8 @@ public class MarkupContainerTest extends WicketTestCase assertThat(iterator1.hasNext(), is(false)); } - /** - * This tests a functional bug in the iterator implementation where you have multiple iterators - * traversing the children, a detach happens and one of the iterators removes a child component - * before the other iterator has a chance to update its internal state to the new world. This is - * a known bug and we expect that this doesn't pose a problem in real world usage. - */ - @Test(expected = ConcurrentModificationException.class) - public void knownBugForDetachWithTwoIteratorsAndRemovals() + @Test + public void detachWithTwoIteratorsAndRemovals() { int n = NUMBER_OF_CHILDREN_FOR_A_MAP * 2; @@ -1160,21 +1154,11 @@ public class MarkupContainerTest extends WicketTestCase iterator1.next(); iterator1.remove(); - // implementation detail that gets in the way of properly solving this exotic use case: at - // this moment iterator 2 doesn't know that the modification count was reset before the - // iterator 1 removed the component. iterator2.next(); - - // code never reaches this point due to the ConcurrentModificationException } - /** - * This test is the working case for the above scenario where two iterators traverse the - * children, the component gets detached and in this case both iterators have a chance to update - * their internal state to the new world, before they continue to traverse the children. - */ @Test - public void detachWithTwoIteratorsAndRemovalsWork() + public void detachWithTwoIteratorsAndRemovals2() { int n = NUMBER_OF_CHILDREN_FOR_A_MAP * 2;
