These ability checks are called very often (eg. cursor paint cycles).
Instead of creating lists of applying abilities first (getAbilities() call)
out of the internal sets and then check for false candidates, we do that
along with the set scan and bail out as soon as possible, thus saving a lot
iterations and allocations.

Nobody overrides these methods, so we don't have to care about possible
incomplete overrides. Just to be sure, mark them final.
---
 .../sf/freecol/common/model/FeatureContainer.java  | 73 +++++++++++++++++-----
 src/net/sf/freecol/common/model/FreeColObject.java |  2 +-
 2 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/src/net/sf/freecol/common/model/FeatureContainer.java 
b/src/net/sf/freecol/common/model/FeatureContainer.java
index 69eec27393c..ca2de20a884 100644
--- a/src/net/sf/freecol/common/model/FeatureContainer.java
+++ b/src/net/sf/freecol/common/model/FeatureContainer.java
@@ -123,9 +123,37 @@ public final class FeatureContainer {
      * @param turn An optional applicable {@code Turn}.
      * @return True if the ability is present.
      */
-    public boolean hasAbility(String id, FreeColSpecObjectType fcgot,
+    public final boolean hasAbility(String id, FreeColSpecObjectType fcgot,
                               int turn) {
-        return FeatureContainer.hasAbility(getAbilities(id, fcgot, turn));
+        if (!abilitiesPresent()) return false;
+
+        boolean ret = false;
+
+        if (id == null) {
+            synchronized (abilitiesLock) {
+                for (Set<Ability> aset : this.abilities.values()) {
+                    for (Ability a : aset) {
+                        if (a.appliesTo(fcgot, turn)) {
+                            if (a.getValue()) ret = true;
+                            else return false;
+                        }
+                    }
+                }
+            }
+            return ret;
+        }
+
+        synchronized (abilitiesLock) {
+            Set<Ability> aset = this.abilities.get(id);
+            if (aset == null) return false;
+            for (Ability a : aset) {
+                if (a.appliesTo(fcgot, turn)) {
+                    if (a.getValue()) ret = true;
+                    else return false;
+                }
+            }
+        }
+        return ret;
     }
 
     /**
@@ -138,23 +166,38 @@ public final class FeatureContainer {
      * @param turn An optional applicable {@code Turn}.
      * @return A stream of abilities.
      */
-    public Stream<Ability> getAbilities(String id, FreeColSpecObjectType fcgot,
+    public final List<Ability> getAbilities(String id, FreeColSpecObjectType 
fcgot,
                                         int turn) {
-        Set<Ability> result = new HashSet<>();
-        if (abilitiesPresent()) {
+        if (!abilitiesPresent())
+            return Collections.<Ability>emptyList();
+
+        if (id == null) {
+            Set<Ability> result = new HashSet<>();
             synchronized (abilitiesLock) {
-                if (id == null) {
-                    for (Set<Ability> aset : abilities.values()) {
-                        result.addAll(aset);
-                    }
-                } else {
-                    Set<Ability> aset = abilities.get(id);
-                    if (aset != null) result.addAll(aset);
-                }
+                // FIXME: check whether we really need the intermediate Set,
+                // iow, whether duplicates may happen and could cause a problem
+                for (Set<Ability> aset : abilities.values())
+                    for (Ability a : aset)
+                        if (a.appliesTo(fcgot, turn))
+                            result.add(a);
             }
-            removeInPlace(result, a -> !a.appliesTo(fcgot, turn));
+            if (result.isEmpty())
+                return Collections.<Ability>emptyList();
+            return new ArrayList<>(result);
+        }
+
+        synchronized (abilitiesLock) {
+            Set<Ability> aset = abilities.get(id);
+            if ((aset == null) || aset.isEmpty())
+                return Collections.<Ability>emptyList();
+
+            List<Ability> result = new ArrayList<>();
+            for (Ability a : aset)
+                if (a.appliesTo(fcgot, turn))
+                    result.add(a);
+
+            return result;
         }
-        return result.stream();
     }
 
     /**
diff --git a/src/net/sf/freecol/common/model/FreeColObject.java 
b/src/net/sf/freecol/common/model/FreeColObject.java
index 6f0b15ada9e..376fd611871 100644
--- a/src/net/sf/freecol/common/model/FreeColObject.java
+++ b/src/net/sf/freecol/common/model/FreeColObject.java
@@ -470,7 +470,7 @@ public abstract class FreeColObject
                                         int turn) {
         FeatureContainer fc = getFeatureContainer();
         return (fc == null) ? Stream.<Ability>empty()
-            : fc.getAbilities(id, fcgot, turn);
+            : fc.getAbilities(id, fcgot, turn).stream();
     }
 
     /**
-- 
2.11.0.rc0.7.gbe5a750


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Freecol-developers mailing list
Freecol-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freecol-developers

Reply via email to