10:28 a.m.
At AS6 meeting we came across some "shocking" ControllerState numbers. :-)
Number of created instances was big,
and the number of equals() and hashCode() invocations was just outrageous ~ 12M.
(if I remember the numbers correctly)
The equals/hash invocations are result of state comparison,
which is done via List::indexOf. (we keep the states in a list)
| protected int getStateIndex(ControllerState state, boolean allowNotFound)
| {
| if (state == null)
| throw new IllegalArgumentException("Null state");
|
| int stateIndex = states.indexOf(state);
| if (stateIndex < 0 && allowNotFound == false)
| throw new IllegalArgumentException("No such state " + state +
" in states " + states);
|
| return stateIndex;
| }
|
So, each time we compare two states, which we do quite a lot,
we're checking it against the list's elements.
This definitely calls for some optimization,
specially as we only expect to have a few diff ControllerState instances.
We should have some CS registry to reuse existing immutable instances,
not to mention changing compare impl.
This is my (very) naive approach, hence some feedback welcome.
| Index: dependency/src/main/java/org/jboss/dependency/plugins/AbstractController.java
| ===================================================================
| ---
dependency/src/main/java/org/jboss/dependency/plugins/AbstractController.java (revision
91575)
| +++
dependency/src/main/java/org/jboss/dependency/plugins/AbstractController.java (working
copy)
| @@ -265,6 +265,8 @@
| if (states.contains(state))
| return;
|
| + state.order(before);
| +
| if (before == null)
| {
| states.add(state);
| @@ -1973,16 +1975,12 @@
|
| public boolean isBeforeState(ControllerState state, ControllerState reference)
| {
| - int stateIndex = getStateIndex(state, true);
| - int referenceIndex = getStateIndex(reference, true);
| - return stateIndex < referenceIndex;
| + return state.compareTo(reference) < 0;
| }
|
| public boolean isAfterState(ControllerState state, ControllerState reference)
| {
| - int stateIndex = getStateIndex(state, true);
| - int referenceIndex = getStateIndex(reference, true);
| - return stateIndex > referenceIndex;
| + return state.compareTo(reference) > 0;
| }
|
| public Iterator<ControllerState> iterator()
| Index: dependency/src/main/java/org/jboss/dependency/spi/ControllerState.java
| ===================================================================
| --- dependency/src/main/java/org/jboss/dependency/spi/ControllerState.java (revision
91575)
| +++ dependency/src/main/java/org/jboss/dependency/spi/ControllerState.java (working
copy)
| @@ -24,7 +24,9 @@
| import java.io.ObjectStreamException;
| import java.io.Serializable;
| import java.util.HashMap;
| +import java.util.List;
| import java.util.Map;
| +import java.util.ArrayList;
|
| import org.jboss.util.JBossObject;
| import org.jboss.util.JBossStringBuilder;
| @@ -33,43 +35,48 @@
| * Description of state.
| *
| * @author <a href="adrian(a)jboss.com">Adrian Brock</a>
| + * @author <a href="ales.justin(a)jboss.com">Ales Justin</a>
| * @version $Revision$
| */
| -public class ControllerState extends JBossObject implements Serializable
| +public class ControllerState extends JBossObject implements Serializable,
Comparable<ControllerState>
| {
| - private static final long serialVersionUID = 2L;
| + private static final long serialVersionUID = 3L;
|
| /** Error */
| - public static final ControllerState ERROR = new
ControllerState("**ERROR**");
| + public static final ControllerState ERROR = new
ControllerState("**ERROR**", -1);
|
| /** Not installed state */
| - public static final ControllerState NOT_INSTALLED = new ControllerState("Not
Installed");
| + public static final ControllerState NOT_INSTALLED = new ControllerState("Not
Installed", 0);
|
| /** Pre install state */
| - public static final ControllerState PRE_INSTALL = new
ControllerState("PreInstall");
| + public static final ControllerState PRE_INSTALL = new
ControllerState("PreInstall", 1);
|
| /** Described state */
| - public static final ControllerState DESCRIBED = new
ControllerState("Described");
| + public static final ControllerState DESCRIBED = new
ControllerState("Described", 2);
|
| /** Instantiated state */
| - public static final ControllerState INSTANTIATED = new
ControllerState("Instantiated");
| + public static final ControllerState INSTANTIATED = new
ControllerState("Instantiated", 3);
|
| /** Configured state */
| - public static final ControllerState CONFIGURED = new
ControllerState("Configured");
| + public static final ControllerState CONFIGURED = new
ControllerState("Configured", 4);
|
| /** Create state */
| - public static final ControllerState CREATE = new
ControllerState("Create");
| + public static final ControllerState CREATE = new
ControllerState("Create", 5);
|
| /** Start state */
| - public static final ControllerState START = new
ControllerState("Start");
| + public static final ControllerState START = new ControllerState("Start",
6);
|
| /** Installed state */
| - public static final ControllerState INSTALLED = new
ControllerState("Installed");
| + public static final ControllerState INSTALLED = new
ControllerState("Installed", 7);
|
| /** The state string */
| protected final String stateString;
|
| + /** The dynamic order */
| + protected int order;
| +
| private static Map<String, ControllerState> values = new HashMap<String,
ControllerState>();
| + private static List<ControllerState> states = new
ArrayList<ControllerState>();
|
| static
| {
| @@ -88,12 +95,15 @@
| * Create a new state
| *
| * @param stateString the string representation
| + * @param order the order
| */
| - public ControllerState(String stateString)
| + private ControllerState(String stateString, int order)
| {
| if (stateString == null)
| throw new IllegalArgumentException("Null state string");
| +
| this.stateString = stateString;
| + this.order = order;
| }
|
| /**
| @@ -110,10 +120,16 @@
| {
| if (object == null || object instanceof ControllerState == false)
| return false;
| +
| ControllerState other = (ControllerState) object;
| - return stateString.equals(other.stateString);
| + return order == other.order;
| }
| -
| +
| + public int compareTo(ControllerState other)
| + {
| + return order - other.order;
| + }
| +
| public void toString(JBossStringBuilder buffer)
| {
| buffer.append(stateString);
| @@ -133,4 +149,56 @@
| {
| return values.get(stateString);
| }
| +
| + /**
| + * Get state instance.
| + *
| + * @param stateString the state string
| + * @return new state instance
| + */
| + public static ControllerState getInstance(String stateString)
| + {
| + if (stateString == null)
| + throw new IllegalArgumentException("Null state string.");
| +
| + ControllerState state = values.get(stateString);
| + if (state != null)
| + return state;
| +
| + state = new ControllerState(stateString, -1);
| + // cache it
| + values.put(stateString, state);
| + return state;
| + }
| +
| + /**
| + * Modify order.
| + *
| + * @param before the before ref
| + */
| + public void order(ControllerState before)
| + {
| + // do string, as we might not be properly ordered yet
| + for (ControllerState cs : states)
| + {
| + if (cs.stateString.equals(stateString))
| + return;
| + }
| +
| + if (before == null)
| + {
| + order = states.size();
| + states.add(this);
| + }
| + else
| + {
| + int index = states.indexOf(before);
| + order = index;
| + // shift others
| + for (int i = index; i < states.size(); i++)
| + states.get(i).order++;
| + // add this state
| + states.add(index, this);
| + }
| + }
| }
|
View the original post :
http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4260336#...
Reply to the post :
http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&a...
6017
days inactive
6017
days old
jboss-dev-forums@lists.jboss.org
0 comments
1 participants
participants (1)
-
alesj