Intern attachment-related strings to minimize memory and speed up clo…

…ning. (#10470)

Intern attachment-related strings to minimize memory and speed up cloning.

Java's String.intern() function returns a canonical String object for the given string,
such that Strings that share content can use the same instance.

This is very relevant for triggers properties, where lots of triggers end up having the same properties set, either via variable expansion or even in older maps, where this was done manually.

This also helps tremendously with GameData cloning, since there's much fewer String objects to clone, since the same String instance is pointed to by multiple objects.

With this change, bringing up the Battle Calculator on "270BC Wars" goes from ~1.5s to ~0.7s on my machine (the 1.5s is already after many other improvements I landed).

Additionally, save game size goes to 860k (from 1.1MB), which is already after compression.
RAM use after GC goes to 129M from 192M.
These two are both after starting an all-human "270BC Wars" game at the start of the first human turn (after all the barbarian combats).

This change also cleans up some unnecessary null checks on attachment setters.

game-app/game-core/src/main/java/games/strategy/engine/data/gameparser/GameParser.java

@@ -961,7 +961,7 @@ private List<Tuple<String, String>> setOptions(
                 xmlUri, attachment, e.getMessage()),
             e);
       }
-      results.add(Tuple.of(name, finalValue));
+      results.add(Tuple.of(name.intern(), finalValue.intern()));
     }
     return results;
   }

game-app/game-core/src/main/java/games/strategy/triplea/attachments/AbstractConditionsAttachment.java

@@ -113,7 +113,7 @@ void setConditionType(final String value) throws GameParseException {
     if (uppercaseValue.matches("AND|OR|\\d+(?:-\\d+)?")) {
       final String[] split = splitOnHyphen(uppercaseValue);
       if (split.length != 2 || Integer.parseInt(split[1]) > Integer.parseInt(split[0])) {
-        conditionType = uppercaseValue;
+        conditionType = uppercaseValue.intern();
         return;
       }
     }
@@ -280,7 +280,7 @@ protected void setChance(final String chance) throws GameParseException {
               + " format: \"1:10\" for 10% chance"
               + thisErrorMsg());
     }
-    this.chance = chance;
+    this.chance = chance.intern();
   }
 
   /**

game-app/game-core/src/main/java/games/strategy/triplea/attachments/AbstractPlayerRulesAttachment.java

@@ -140,7 +140,7 @@ private void setMovementRestrictionType(final String value) throws GameParseExce
       throw new GameParseException(
           "movementRestrictionType must be allowed or disallowed" + thisErrorMsg());
     }
-    movementRestrictionType = value;
+    movementRestrictionType = value.intern();
   }
 
   public @Nullable String getMovementRestrictionType() {

game-app/game-core/src/main/java/games/strategy/triplea/attachments/AbstractRulesAttachment.java

@@ -161,7 +161,7 @@ private void resetSwitch() {
   }
 
   private void setGameProperty(final String value) {
-    gameProperty = value;
+    gameProperty = value.intern();
   }
 
   private @Nullable String getGameProperty() {
@@ -341,6 +341,9 @@ protected void validateNames(final String[] terrList) {
     if (terrList != null && terrList.length > 0) {
       getListedTerritories(terrList, true, true);
       // removed checks for length & group commands because it breaks the setTerritoryCount feature.
+      for (int i = 0; i < terrList.length; i++) {
+        terrList[i] = terrList[i].intern();
+      }
     }
   }
 

game-app/game-core/src/main/java/games/strategy/triplea/attachments/AbstractTriggerAttachment.java

@@ -133,7 +133,7 @@ private void setWhen(final String when) throws GameParseException {
     if (this.when == null) {
       this.when = new ArrayList<>();
     }
-    this.when.add(Tuple.of(s[0], s[1]));
+    this.when.add(Tuple.of(s[0].intern(), s[1].intern()));
   }
 
   private void setWhen(final List<Tuple<String, String>> value) {
@@ -261,7 +261,7 @@ protected static String getValueFromStringArrayForAllExceptLastSubstring(final S
     if (sb.length() > 0 && sb.substring(0, 1).equals(":")) {
       sb.replace(0, 1, "");
     }
-    return sb.toString();
+    return sb.toString().intern();
   }
 
   public static int getEachMultiple(final AbstractTriggerAttachment t) {

game-app/game-core/src/main/java/games/strategy/triplea/attachments/AbstractUserActionAttachment.java

@@ -59,7 +59,7 @@ public boolean canPerform(final Map<ICondition, Boolean> testedConditions) {
   }
 
   private void setText(final String text) {
-    this.text = text;
+    this.text = text.intern();
   }
 
   /**

game-app/game-core/src/main/java/games/strategy/triplea/attachments/CanalAttachment.java

@@ -73,7 +73,7 @@ static CanalAttachment get(final Territory t, final String nameOfAttachment) {
   }
 
   private void setCanalName(final String name) {
-    canalName = name;
+    canalName = name.intern();
   }
 
   public String getCanalName() {
@@ -85,10 +85,6 @@ private void resetCanalName() {
   }
 
   private void setLandTerritories(final String landTerritories) {
-    if (landTerritories == null) {
-      this.landTerritories = null;
-      return;
-    }
     final Set<Territory> terrs = new HashSet<>();
     for (final String name : splitOnColon(landTerritories)) {
       final Territory territory = getData().getMap().getTerritory(name);

game-app/game-core/src/main/java/games/strategy/triplea/attachments/PlayerAttachment.java

@@ -164,7 +164,7 @@ private Triple<Integer, String, Set<UnitType>> parseUnitLimit(
         types.add(getUnitTypeOrThrow(s[i]));
       }
     }
-    return Triple.of(max, s[1], types);
+    return Triple.of(max, s[1].intern(), types);
   }
 
   /**

game-app/game-core/src/main/java/games/strategy/triplea/attachments/PoliticalActionAttachment.java

@@ -131,7 +131,7 @@ void setRelationshipChange(final String relChange) throws GameParseException {
     if (relationshipChange == null) {
       relationshipChange = new ArrayList<>();
     }
-    relationshipChange.add(relChange);
+    relationshipChange.add(relChange.intern());
   }
 
   private void setRelationshipChange(final List<String> value) {

game-app/game-core/src/main/java/games/strategy/triplea/attachments/RelationshipTypeAttachment.java

@@ -74,7 +74,7 @@ public void setArcheType(final String archeType) throws GameParseException {
       case ARCHETYPE_WAR:
       case ARCHETYPE_ALLIED:
       case ARCHETYPE_NEUTRAL:
-        this.archeType = lowerArcheType;
+        this.archeType = lowerArcheType.intern();
         break;
       default:
         throw new GameParseException(
@@ -109,7 +109,7 @@ private void resetArcheType() {
    * @param canFlyOver should be "true", "false" or "default"
    */
   private void setCanMoveAirUnitsOverOwnedLand(final String canFlyOver) {
-    canMoveAirUnitsOverOwnedLand = canFlyOver;
+    canMoveAirUnitsOverOwnedLand = canFlyOver.intern();
   }
 
   private String getCanMoveAirUnitsOverOwnedLand() {
@@ -135,7 +135,7 @@ private void resetCanMoveAirUnitsOverOwnedLand() {
   }
 
   private void setCanMoveLandUnitsOverOwnedLand(final String canFlyOver) {
-    canMoveLandUnitsOverOwnedLand = canFlyOver;
+    canMoveLandUnitsOverOwnedLand = canFlyOver.intern();
   }
 
   private String getCanMoveLandUnitsOverOwnedLand() {
@@ -154,7 +154,7 @@ private void resetCanMoveLandUnitsOverOwnedLand() {
   }
 
   private void setCanLandAirUnitsOnOwnedLand(final String canLandAir) {
-    canLandAirUnitsOnOwnedLand = canLandAir;
+    canLandAirUnitsOnOwnedLand = canLandAir.intern();
   }
 
   private String getCanLandAirUnitsOnOwnedLand() {
@@ -174,7 +174,7 @@ private void resetCanLandAirUnitsOnOwnedLand() {
   }
 
   private void setCanTakeOverOwnedTerritory(final String canTakeOver) {
-    canTakeOverOwnedTerritory = canTakeOver;
+    canTakeOverOwnedTerritory = canTakeOver.intern();
   }
 
   private String getCanTakeOverOwnedTerritory() {
@@ -222,7 +222,7 @@ private void setUpkeepCost(final String integerCost) throws GameParseException {
                     + thisErrorMsg());
         }
       }
-      upkeepCost = integerCost;
+      upkeepCost = integerCost.intern();
     }
   }
 
@@ -250,7 +250,7 @@ private void setAlliancesCanChainTogether(final String value) throws GameParseEx
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    alliancesCanChainTogether = value;
+    alliancesCanChainTogether = value.intern();
   }
 
   private String getAlliancesCanChainTogether() {
@@ -281,7 +281,7 @@ private void setIsDefaultWarPosition(final String value) throws GameParseExcepti
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    isDefaultWarPosition = value;
+    isDefaultWarPosition = value.intern();
   }
 
   private String getIsDefaultWarPosition() {
@@ -312,7 +312,7 @@ private void setGivesBackOriginalTerritories(final String value) throws GamePars
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    givesBackOriginalTerritories = value;
+    givesBackOriginalTerritories = value.intern();
   }
 
   private String getGivesBackOriginalTerritories() {
@@ -341,7 +341,7 @@ private void setCanMoveIntoDuringCombatMove(final String value) throws GameParse
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    canMoveIntoDuringCombatMove = value;
+    canMoveIntoDuringCombatMove = value.intern();
   }
 
   private String getCanMoveIntoDuringCombatMove() {
@@ -371,7 +371,7 @@ private void setCanMoveThroughCanals(final String value) throws GameParseExcepti
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    canMoveThroughCanals = value;
+    canMoveThroughCanals = value.intern();
   }
 
   private String getCanMoveThroughCanals() {
@@ -403,7 +403,7 @@ private void setRocketsCanFlyOver(final String value) throws GameParseException
               + PROPERTY_TRUE
               + thisErrorMsg());
     }
-    rocketsCanFlyOver = value;
+    rocketsCanFlyOver = value.intern();
   }
 
   private String getRocketsCanFlyOver() {

game-app/game-core/src/main/java/games/strategy/triplea/attachments/RulesAttachment.java

@@ -164,7 +164,7 @@ private void setDestroyedTuv(final String value) throws GameParseException {
       throw new GameParseException(
           "destroyedTUV value must be currentRound or allRounds" + thisErrorMsg());
     }
-    destroyedTuv = value;
+    destroyedTuv = value.intern();
   }
 
   private @Nullable String getDestroyedTuv() {
@@ -216,7 +216,7 @@ private void setBattle(final String value) throws GameParseException {
     if (battle == null) {
       battle = new ArrayList<>();
     }
-    battle.add(Tuple.of((s[0] + ":" + s[1] + ":" + s[2] + ":" + s[3]), terrs));
+    battle.add(Tuple.of((s[0] + ":" + s[1] + ":" + s[2] + ":" + s[3]).intern(), terrs));
   }
 
   private void setBattle(final List<Tuple<String, List<Territory>>> value) {
@@ -280,7 +280,8 @@ private void setRelationship(final String value) throws GameParseException {
     if (relationship == null) {
       relationship = new ArrayList<>();
     }
-    relationship.add((s.length == 3) ? (value + ":-1") : value);
+    String str = (s.length == 3) ? (value + ":-1") : value;
+    relationship.add(str.intern());
   }
 
   private void setRelationship(final List<String> value) {
@@ -474,7 +475,7 @@ private void setUnitPresence(final String value) throws GameParseException {
     if (unitPresence == null) {
       unitPresence = new IntegerMap<>();
     }
-    unitPresence.put(value.replaceFirst(s[0] + ":", ""), n);
+    unitPresence.put(value.replaceFirst(s[0] + ":", "").intern(), n);
   }
 
   private void setUnitPresence(final IntegerMap<String> value) {
@@ -498,10 +499,6 @@ private int getTechCount() {
   }
 
   private void setAtWarPlayers(final String players) throws GameParseException {
-    if (players == null) {
-      atWarPlayers = null;
-      return;
-    }
     final String[] s = splitOnColon(players);
     if (s.length < 1) {
       throw new GameParseException("Empty enemy list" + thisErrorMsg());
@@ -535,10 +532,6 @@ private void resetAtWarPlayers() {
   }
 
   private void setTechs(final String newTechs) throws GameParseException {
-    if (newTechs == null) {
-      techs = null;
-      return;
-    }
     final String[] s = splitOnColon(newTechs);
     if (s.length < 1) {
       throw new GameParseException("Empty tech list" + thisErrorMsg());

game-app/game-core/src/main/java/games/strategy/triplea/attachments/TechAbilityAttachment.java

@@ -511,7 +511,7 @@ private void setUnitAbilitiesGained(final String value) throws GameParseExceptio
                 + ABILITY_CAN_BOMBARD
                 + thisErrorMsg());
       }
-      abilities.add(ability);
+      abilities.add(ability.intern());
     }
   }
 
@@ -673,7 +673,7 @@ private void setAirborneTargettedByAa(final String value) throws GameParseExcept
     if (airborneTargetedByAa == null) {
       airborneTargetedByAa = new HashMap<>();
     }
-    airborneTargetedByAa.put(s[0], unitTypes);
+    airborneTargetedByAa.put(s[0].intern(), unitTypes);
   }
 
   private void setAirborneTargettedByAa(final Map<String, Set<UnitType>> value) {

game-app/game-core/src/main/java/games/strategy/triplea/attachments/TechAttachment.java

@@ -334,7 +334,7 @@ public boolean getShipyards() {
   private void setGenericTechs() {
     for (final TechAdvance ta : getData().getTechnologyFrontier()) {
       if (ta instanceof GenericTechAdvance && ((GenericTechAdvance) ta).getAdvance() == null) {
-        genericTech.put(ta.getProperty(), Boolean.FALSE);
+        genericTech.put(ta.getProperty().intern(), Boolean.FALSE);
       }
     }
   }

game-app/game-core/src/main/java/games/strategy/triplea/attachments/TerritoryAttachment.java

@@ -430,7 +430,7 @@ void setWhenCapturedByGoesTo(final String value) throws GameParseException {
     if (whenCapturedByGoesTo == null) {
       whenCapturedByGoesTo = new ArrayList<>();
     }
-    whenCapturedByGoesTo.add(value);
+    whenCapturedByGoesTo.add(value.intern());
   }
 
   private void setWhenCapturedByGoesTo(final List<String> value) {