BUG-582: optimize InstanceIdentifier.firstIdentifierOf()

Performance tracing of OpenFlow components found that we are spending
cycles copying arguments in create() when called from firstIdentifierOf().
This does not make sense, as we know the argument list is immutable in
this code path.

As it turns out, the copy guard in create() is not sufficient to prevent
the copy, as the Iterables.limit() instantiates a FluentIterable, not an
ImmutableCollection.

This patch elides the copy by splitting the non-copy part of logic of
create() to a private method, internalCreate(). firstIdentifierOf()
calls this method directly, while create() calls it after potentially
creating a copy.

Change-Id: I5c3dbb5c6b5e26d30a0bb407822f1d40fcc898f5
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/yang/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java b/yang/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java
index b5ae91f7e9563a5196b43f28b169c11b9f3c2878..be97e1fd47034df5e1adb80859b5fa9ca9e38466 100644 (file)
--- a/yang/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java
+++ b/yang/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/InstanceIdentifier.java
@@ -193,7 +193,7 @@ public class InstanceIdentifier<T extends DataObject> implements Path<InstanceId
         for (final PathArgument a : getPathArguments()) {
             if (type.equals(a.getType())) {
                 @SuppressWarnings("unchecked")
-                final InstanceIdentifier<I> ret = (InstanceIdentifier<I>) create(Iterables.limit(getPathArguments(), i));
+                final InstanceIdentifier<I> ret = (InstanceIdentifier<I>) internalCreate(Iterables.limit(getPathArguments(), i));
                 return ret;
             }
 
@@ -385,20 +385,17 @@ public class InstanceIdentifier<T extends DataObject> implements Path<InstanceId
     }
 
     /**
-     * Create an instance identifier for a very specific object type.
+     * Create an instance identifier for a very specific object type. This method
+     * implements {@link #create(Iterable)} semantics, except it is used by internal
+     * callers, which have assured that the argument is an immutable Iterable.
      *
-     * Example
-     * <pre>
-     *  List<PathArgument> path = Arrays.asList(new Item(Nodes.class))
-     *  new InstanceIdentifier(path);
-     * </pre>
      *
      * @param pathArguments The path to a specific node in the data tree
      * @return InstanceIdentifier instance
      * @throws IllegalArgumentException if pathArguments is empty or
      *         contains a null element.
      */
-    public static InstanceIdentifier<?> create(final Iterable<? extends PathArgument> pathArguments) {
+    private static InstanceIdentifier<?> internalCreate(final Iterable<PathArgument> pathArguments) {
         final Iterator<? extends PathArgument> it = Preconditions.checkNotNull(pathArguments, "pathArguments may not be null").iterator();
         final HashCodeBuilder<PathArgument> hashBuilder = new HashCodeBuilder<>();
         boolean wildcard = false;
@@ -417,14 +414,31 @@ public class InstanceIdentifier<T extends DataObject> implements Path<InstanceId
         }
         Preconditions.checkArgument(a != null, "pathArguments may not be empty");
 
-        final Iterable<PathArgument> immutableArguments;
+        return trustedCreate(a, pathArguments, hashBuilder.toInstance(), wildcard);
+    }
+
+    /**
+     * Create an instance identifier for a very specific object type.
+     *
+     * Example
+     * <pre>
+     *  List<PathArgument> path = Arrays.asList(new Item(Nodes.class))
+     *  new InstanceIdentifier(path);
+     * </pre>
+     *
+     * @param pathArguments The path to a specific node in the data tree
+     * @return InstanceIdentifier instance
+     * @throws IllegalArgumentException if pathArguments is empty or
+     *         contains a null element.
+     */
+    public static InstanceIdentifier<?> create(final Iterable<? extends PathArgument> pathArguments) {
         if (pathArguments instanceof ImmutableCollection<?>) {
-            immutableArguments = (Iterable<PathArgument>) pathArguments;
+            @SuppressWarnings("unchecked")
+            final Iterable<PathArgument> immutableArguments = (Iterable<PathArgument>) pathArguments;
+            return internalCreate(immutableArguments);
         } else {
-            immutableArguments = ImmutableList.copyOf(pathArguments);
+            return internalCreate(ImmutableList.copyOf(pathArguments));
         }
-
-        return trustedCreate(a, immutableArguments, hashBuilder.toInstance(), wildcard);
     }
 
     /**