From: Tom Pantelis <tpanteli@brocade.com>
Date: Thu, 1 Oct 2015 07:11:28 +0000 (-0400)
Subject: Fix performance issue in EffectiveStatementBase
X-Git-Tag: release/beryllium~240
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=69dcbda23d8cf16be62233558d98b9d43ee618dd;p=yangtools.git

Fix performance issue in EffectiveStatementBase

While running some feature tests I took some thread dumps and noticed
the following stack trace in each:

java.lang.Thread.State: RUNNABLE
        at java.lang.Throwable.fillInStackTrace(Native Method)
        at java.lang.Throwable.fillInStackTrace(Throwable.java:783)
        - locked <0x00000000e5aff880> (a
          java.util.NoSuchElementException)
        at java.lang.Throwable.<init>(Throwable.java:250)
        at java.lang.Exception.<init>(Exception.java:41)
        at java.lang.RuntimeException.<init>(RuntimeException.java:51)
        at
java.util.NoSuchElementException.<init>(NoSuchElementException.java:47)
        at
com.google.common.collect.AbstractIterator.next(AbstractIterator.java:154)
        at com.google.common.collect.Iterators.find(Iterators.java:717)
        at com.google.common.collect.Iterables.find(Iterables.java:646)
        at
org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective.EffectiveStatementBase.firstEffective(EffectiveStatementBase.java:105)

The problem is that Iterables.find throws NoSuchElementException if not
found which is expensive when called many times. I changed it to call
tryFind instead which returns an Optional and this significantly
improved performance. The test time went from 15 min to 3 min.

There may be other classes that call Iterables.find - I didn't check.
If so they can be fixed in subsequent patches.

Change-Id: I0b3d02979b6d7fc97752c4b2429720b0807b853b
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
---

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
index 35dd11c511..d3b190ce4a 100644
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
@@ -7,16 +7,19 @@
  */
 package org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective;
 
+import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.Map;
 import java.util.NoSuchElementException;
+
 import org.opendaylight.yangtools.yang.model.api.Rfc6020Mapping;
 import org.opendaylight.yangtools.yang.model.api.SchemaNode;
 import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
@@ -100,23 +103,15 @@ abstract public class EffectiveStatementBase<A, D extends DeclaredStatement<A>>
     }
 
     protected final <S extends EffectiveStatement<?, ?>> S firstEffective(final Class<S> type) {
-        S result = null;
-        try {
-            result = type.cast(Iterables.find(substatements, Predicates.instanceOf(type)));
-        } catch (NoSuchElementException e) {
-            result = null;
-        }
-        return result;
+        Optional<? extends EffectiveStatement<?, ?>> possible = Iterables.tryFind(substatements,
+                Predicates.instanceOf(type));
+        return possible.isPresent() ? type.cast(possible.get()) : null;
     }
 
     protected final <S extends SchemaNode> S firstSchemaNode(final Class<S> type) {
-        S result = null;
-        try {
-            result = type.cast(Iterables.find(substatements, Predicates.instanceOf(type)));
-        } catch (NoSuchElementException e) {
-            result = null;
-        }
-        return result;
+        Optional<? extends EffectiveStatement<?, ?>> possible = Iterables.tryFind(substatements,
+                Predicates.instanceOf(type));
+        return possible.isPresent() ? type.cast(possible.get()) : null;
     }
 
     @SuppressWarnings("unchecked")
@@ -132,23 +127,14 @@ abstract public class EffectiveStatementBase<A, D extends DeclaredStatement<A>>
     }
 
     protected final <T> T firstSubstatementOfType(final Class<T> type) {
-        T result = null;
-        try {
-            result = type.cast(Iterables.find(substatements, Predicates.instanceOf(type)));
-        } catch (NoSuchElementException e) {
-            result = null;
-        }
-        return result;
+        Optional<? extends EffectiveStatement<?, ?>> possible = Iterables.tryFind(substatements,
+                Predicates.instanceOf(type));
+        return possible.isPresent() ? type.cast(possible.get()) : null;
     }
 
     protected final <R> R firstSubstatementOfType(final Class<?> type, final Class<R> returnType) {
-        R result = null;
-        try {
-            result = returnType.cast(Iterables.find(substatements,
-                    Predicates.and(Predicates.instanceOf(type), Predicates.instanceOf(returnType))));
-        } catch (NoSuchElementException e) {
-            result = null;
-        }
-        return result;
+        Optional<? extends EffectiveStatement<?, ?>> possible = Iterables.tryFind(substatements,
+                Predicates.and(Predicates.instanceOf(type), Predicates.instanceOf(returnType)));
+        return possible.isPresent() ? returnType.cast(possible.get()) : null;
     }
 }