Skip to content

Commit

Permalink
CLDR-17314 String.intern() across multiple locations
Browse files Browse the repository at this point in the history
- Unit test to verify that CLDRFile paths are interned
- interning in XPathTable and StringId
  • Loading branch information
srl295 committed Jan 23, 2024
1 parent 818a29c commit 145b379
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ private synchronized void addXpaths(Set<String> xpaths, Connection conn) throws
* @return the xpath's id (as an Integer)
*/
private synchronized Integer addXpath(String xpath, boolean addIfNotFound, Connection inConn) {
xpath = xpath.intern();
Integer nid = stringToId.get(xpath); // double check
if (nid != null) {
return nid;
Expand Down Expand Up @@ -372,6 +373,7 @@ public final String getById(int id) {
* //ldml/dates/timeZoneNames/zone[@type="America/Guadeloupe"]/short/daylight
*/
public final void setById(int id, String xpath) {
xpath = xpath.intern();
stringToId.put(idToString_put(id, xpath), id);
sidToString.put(getStringID(xpath), xpath);
}
Expand All @@ -383,6 +385,7 @@ public final void setById(int id, String xpath) {
* @return the id, like 692804, for the specified path
*/
public final int getByXpath(String xpath) {
xpath = xpath.intern();
Integer nid = stringToId.get(xpath);
if (nid != null) {
return nid.intValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3156,8 +3156,8 @@ public Set<String> getRawExtraPaths() {
if (extraPaths == null) {
extraPaths =
ImmutableSet.<String>builder()
.addAll(getRawExtraPathsPrivate())
.addAll(CONST_EXTRA_PATHS)
.addAll(CharUtilities.internAll(getRawExtraPathsPrivate()))
.addAll(CharUtilities.internAll(CONST_EXTRA_PATHS))
.build();
if (DEBUG) {
System.out.println(getLocaleID() + "\textras: " + extraPaths.size());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,31 @@
package org.unicode.cldr.util;

import java.util.Iterator;

public class CharUtilities {
/** intern everything in the src iterable */
public static Iterable<String> internAll(final Iterable<String> src) {
return new Iterable<String>() {
@Override
public Iterator<String> iterator() {
return CharUtilities.internAll(src.iterator());
}
};
}
/** intern everything in the src iterator */
protected static Iterator<String> internAll(final Iterator<String> iterator) {
return new Iterator<String>() {
@Override
public boolean hasNext() {
return iterator.hasNext();
}

@Override
public String next() {
return iterator.next().intern();
}
};
}
/**
* Simple wrapper for CharSequence
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,12 @@ public XMLSource cloneAsThawed() {

@Override
public void putFullPathAtDPath(String distinguishingXPath, String fullxpath) {
xpath_fullXPath.put(distinguishingXPath, fullxpath);
xpath_fullXPath.put(distinguishingXPath.intern(), fullxpath.intern());
}

@Override
public void putValueAtDPath(String distinguishingXPath, String value) {
distinguishingXPath = distinguishingXPath.intern();
String oldValue = xpath_value.get(distinguishingXPath);
xpath_value.put(distinguishingXPath, value);
updateValuePathMapping(distinguishingXPath, oldValue, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public final class StringId {
* @return a value from 0 to 0x7FFFFFFFFFFFFFFFL.
*/
public static long getId(CharSequence charSequence) {
String string = charSequence.toString();
String string = charSequence.toString().intern();
Long resultLong = STRING_TO_ID.get(string);
if (resultLong != null) {
return resultLong;
Expand Down
19 changes: 12 additions & 7 deletions tools/cldr-code/src/main/java/org/unicode/cldr/util/XMLSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ public boolean isFrozen() {
* @param value
*/
public String putValueAtPath(String xpath, String value) {
xpath = xpath.intern();
if (locked) {
throw new UnsupportedOperationException("Attempt to modify locked object");
}
Expand Down Expand Up @@ -493,7 +494,8 @@ private TreeMap<String, String> loadAliases() {
if (!Alias.isAliasPath(path)) {
continue;
}
String fullPath = getFullPathAtDPath(path);
path = path.intern();
String fullPath = getFullPathAtDPath(path).intern();
Alias temp = Alias.make(fullPath);
if (temp == null) {
continue;
Expand Down Expand Up @@ -892,6 +894,7 @@ public Date getChangeDateAtDPath(String xpath) {
private String getFullPath(
String xpath, AliasLocation fullStatus, String fullPathWhereFound) {
String result = null;
xpath = xpath.intern();
if (this.cachingIsEnabled) {
result = getFullPathAtDPathCache.get(xpath);
}
Expand Down Expand Up @@ -1078,6 +1081,7 @@ private AliasLocation getPathLocation(
boolean skipFirst,
boolean skipInheritanceMarker,
List<LocaleInheritanceInfo> list) {
xpath = xpath.intern();

// When calculating the Bailey values, we track the final
// return value as firstValue. If non-null, this will become
Expand Down Expand Up @@ -1145,6 +1149,7 @@ private AliasLocation getPathLocation(
aliasedPath =
aliases.get(possibleSubpath)
+ xpath.substring(possibleSubpath.length());
aliasedPath = aliasedPath.intern();
if (list != null) {
// It's an explicit alias, just at a parent element (subset xpath)
list.add(
Expand All @@ -1165,7 +1170,7 @@ private AliasLocation getPathLocation(

// alts are special; they act like there is a root alias to the path without the alt.
if (aliasedPath == null && xpath.contains("[@alt=")) {
aliasedPath = XPathParts.getPathWithoutAlt(xpath);
aliasedPath = XPathParts.getPathWithoutAlt(xpath).intern();
if (list != null) {
list.add(
new LocaleInheritanceInfo(
Expand All @@ -1178,10 +1183,10 @@ private AliasLocation getPathLocation(
// //ldml/numbers/currencies/currency[@type="BRZ"]/displayName[@count="other"] =>
// //ldml/numbers/currencies/currency[@type="BRZ"]/displayName
if (aliasedPath == null && xpath.contains("[@count=")) {
aliasedPath = COUNT_EQUALS.matcher(xpath).replaceAll("[@count=\"other\"]");
aliasedPath = COUNT_EQUALS.matcher(xpath).replaceAll("[@count=\"other\"]").intern();
if (aliasedPath.equals(xpath)) {
if (xpath.contains("/displayName")) {
aliasedPath = COUNT_EQUALS.matcher(xpath).replaceAll("");
aliasedPath = COUNT_EQUALS.matcher(xpath).replaceAll("").intern();
if (aliasedPath.equals(xpath)) {
throw new RuntimeException("Internal error");
}
Expand Down Expand Up @@ -1290,7 +1295,7 @@ private Set<String> findNonAliasedPaths() {
// others
for (XMLSource curSource : sourceList) {
for (String xpath : curSource) {
paths.add(xpath);
paths.add(xpath.intern());
}
}
return paths;
Expand Down Expand Up @@ -1328,7 +1333,7 @@ private Set<String> getDirectAliases(String[] paths) {
String suffix = xpath.substring(suffixStart);
for (String reverseAlias : list) {
String reversePath = reverseAlias + suffix;
newPaths.add(reversePath);
newPaths.add(reversePath.intern());
}
endIndex++;
}
Expand Down Expand Up @@ -1967,7 +1972,7 @@ public XMLSource add(String currentFullXPath, String value) {
LOG_PROGRESS,
"ADDING: \t" + currentFullXPath + " \t" + value + "\t" + currentFullXPath);
try {
this.putValueAtPath(currentFullXPath, value);
this.putValueAtPath(currentFullXPath.intern(), value);
} catch (RuntimeException e) {
throw new IllegalArgumentException(
"failed adding " + currentFullXPath + ",\t" + value, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,24 @@ public void testGetPaths() {
.getPathsWhereFound(GERMAN_IN_SWITZERLAND));
}
}

@Test
public void TestInternedPaths() {
{
final CLDRFile en = CLDRConfig.getInstance().getCLDRFile("en", false);
for (final String s : en.fullIterable()) {
final String s0 = new String(s);
assertTrue(s != s0);
assertTrue(s == s0.intern(), () -> "in unresolved en was not interned: " + s);
}
}
{
final CLDRFile en = CLDRConfig.getInstance().getCLDRFile("en", true);
for (final String s : en.fullIterable()) {
final String s0 = new String(s);
assertTrue(s != s0);
assertTrue(s == s0.intern(), () -> "in resolved en was not interned: " + s);
}
}
}
}

0 comments on commit 145b379

Please sign in to comment.