You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by da...@apache.org on 2015/09/21 13:06:07 UTC

[1/3] wicket git commit: Documented the children field and map threshold

Repository: wicket
Updated Branches:
  refs/heads/WICKET-5981 26cecdc6f -> 2246156fc


Documented the children field and map threshold


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/7be920d4
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/7be920d4
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/7be920d4

Branch: refs/heads/WICKET-5981
Commit: 7be920d4403d719e26d2a131454928c086a7317c
Parents: 26cecdc
Author: Martijn Dashorst <ma...@gmail.com>
Authored: Mon Sep 21 13:01:41 2015 +0200
Committer: Martijn Dashorst <ma...@gmail.com>
Committed: Mon Sep 21 13:01:41 2015 +0200

----------------------------------------------------------------------
 .../java/org/apache/wicket/MarkupContainer.java    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/7be920d4/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 c25c3db..4571c29 100644
--- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
@@ -100,13 +100,26 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp
 	private static final long serialVersionUID = 1L;
 	
 	private static final int INITIAL_CHILD_LIST_CAPACITY = 12;
-	
+
+	/**
+	 * The threshold where we start using a Map to store children in, replacing a List. Adding
+	 * components to a list is O(n), and to a map O(1). The magic number is 24, due to a Map using
+	 * more memory to store its elements and below 24 children there's no discernible difference
+	 * between adding to a Map or a List.
+	 * 
+	 * We have focused on adding elements to a list, instead of indexed lookups because adding is an
+	 * action that is performed very often, and lookups often are done by component IDs, not index.
+	 */
 	private static final int MAPIFY_THRESHOLD = 24; // 32 * 0.75
 
 	/** Log for reporting. */
 	private static final Logger log = LoggerFactory.getLogger(MarkupContainer.class);
 
-	/** List of children or single child */
+	/**
+	 * The children of this markup container, if any. Can be a Component when there's only one
+	 * child, a List when the number of children is fewer than {@link #MAPIFY_THRESHOLD} or a Map
+	 * when there are more children.
+	 */
 	private Object children;
 
 	/**


[2/3] wicket git commit: Improved child already exists error message

Posted by da...@apache.org.
Improved child already exists error message

Added the type of the previously existing component.


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/76cd2266
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/76cd2266
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/76cd2266

Branch: refs/heads/WICKET-5981
Commit: 76cd22665ff2eb572ea9165af4581050fd304beb
Parents: 7be920d
Author: Martijn Dashorst <ma...@gmail.com>
Authored: Mon Sep 21 13:02:24 2015 +0200
Committer: Martijn Dashorst <ma...@gmail.com>
Committed: Mon Sep 21 13:02:24 2015 +0200

----------------------------------------------------------------------
 .../src/main/java/org/apache/wicket/MarkupContainer.java  | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/76cd2266/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 4571c29..8bbb1fa 100644
--- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
@@ -194,11 +194,13 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp
 				parent.remove(child);
 			}
 
-			// Add to map
-			if (put(child) != null)
+			// Add the child to my children 
+			Component previousChild = put(child);
+			if (previousChild != null)
 			{
-				throw new IllegalArgumentException(exceptionMessage("A child with id '"
-					+ child.getId() + "' already exists"));
+				throw new IllegalArgumentException(
+					exceptionMessage("A child '" + previousChild.getClass().getSimpleName() +
+						"' with id '" + child.getId() + "' already exists"));
 			}
 
 			addedComponent(child);


[3/3] wicket git commit: Improved the queued components lookup

Posted by da...@apache.org.
Improved the queued components lookup

The queued components lookup back up the tree is now optimized such
that it only looks up until the queue region is found, and only
dequeues when there are components to dequeue.


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/2246156f
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/2246156f
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/2246156f

Branch: refs/heads/WICKET-5981
Commit: 2246156fc61abec16cd3f4f1be21dcd59be0aa16
Parents: 76cd226
Author: Martijn Dashorst <ma...@gmail.com>
Authored: Mon Sep 21 13:04:10 2015 +0200
Committer: Martijn Dashorst <ma...@gmail.com>
Committed: Mon Sep 21 13:04:10 2015 +0200

----------------------------------------------------------------------
 .../java/org/apache/wicket/MarkupContainer.java | 38 ++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/2246156f/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 8bbb1fa..d7da398 100644
--- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
+++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
@@ -1715,16 +1715,34 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp
 		else
 		{
 			MarkupContainer containerWithQueue = this;
-			while (containerWithQueue.queue == null || containerWithQueue.queue.isEmpty())
+
+			// check if there are any parent containers that have queued components, up till our
+			// queue region
+			while (containerWithQueue.isQueueEmpty() &&
+				!(containerWithQueue instanceof IQueueRegion))
 			{
 				containerWithQueue = containerWithQueue.getParent();
 				if (containerWithQueue == null)
 				{
+					// no queued components are available for dequeuing, so we can stop
 					return;
 				}
 			}
 
-			MarkupContainer queueRegion = (MarkupContainer)findParent(IQueueRegion.class);
+			// when there are no components to be dequeued, just stop
+			if (containerWithQueue.isQueueEmpty())
+				return;
+
+			// get the queue region where we are going to dequeue components in
+			MarkupContainer queueRegion = containerWithQueue;
+
+			// the container with queued components could be a queue region, if not, find the region
+			// to dequeue in
+			if (!queueRegion.isQueueRegion())
+			{
+				queueRegion = (MarkupContainer)queueRegion.findParent(IQueueRegion.class);
+			}
+
 			if (queueRegion != null && !queueRegion.getRequestFlag(RFLAG_CONTAINER_DEQUEING))
 			{
 				queueRegion.dequeue();
@@ -1733,6 +1751,22 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp
 	}
 
 	/**
+	 * @return {@code true} when one or more components are queued
+	 */
+	private boolean isQueueEmpty()
+	{
+		return queue == null || queue.isEmpty();
+	}
+
+	/**
+	 * @return {@code true} when this markup container is a queue region
+	 */
+	private boolean isQueueRegion() 
+	{
+		return IQueueRegion.class.isInstance(this);
+	}
+
+	/**
 	 * Run preliminary operations before running {@link #dequeue(DequeueContext)}. More in detail it
 	 * throws an exception if the container is already dequeuing, and it also takes care of setting
 	 * flag {@code RFLAG_CONTAINER_DEQUEING} to true before running {@link #dequeue(DequeueContext)}