Skip to content

Commit 96b543b

Browse files
HenrikPaetzoldHenrikPaetzold
authored
Performance and correctness fixes (#1767)
1 parent 6873ee4 commit 96b543b

8 files changed

Lines changed: 101 additions & 67 deletions

File tree

‎src/main/java/org/rumbledb/context/NamedFunctions.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
import org.rumbledb.runtime.functions.FunctionItemCallIterator;
3737
import org.rumbledb.runtime.typing.AtMostOneItemTypePromotionIterator;
3838
import org.rumbledb.runtime.typing.TypePromotionIterator;
39+
import org.rumbledb.types.BuiltinTypesCatalogue;
3940
import org.rumbledb.types.FunctionSignature;
41+
import org.rumbledb.types.ItemType;
4042
import org.rumbledb.types.ItemTypeFactory;
4143
import org.rumbledb.types.SequenceType;
4244
import org.rumbledb.types.SequenceType.Arity;
@@ -238,6 +240,9 @@ public static RuntimeIterator getBuiltInFunctionIterator(
238240
.equals(SequenceType.createSequenceType("item*"))
239241
) {
240242
SequenceType sequenceType = builtinFunction.getSignature().getParameterTypes().get(i);
243+
if (conversionIsIdentity(arguments.get(i).getStaticType(), sequenceType)) {
244+
continue;
245+
}
241246
RuntimeStaticContext argStaticContext = callerStaticContext
242247
.toBuilder()
243248
.staticType(sequenceType)
@@ -353,7 +358,22 @@ public static RuntimeIterator getBuiltInFunctionIterator(
353358
);
354359
}
355360

356-
361+
private static boolean conversionIsIdentity(SequenceType argumentType, SequenceType expectedType) {
362+
if (argumentType == null || !argumentType.isResolved() || !expectedType.isResolved()) {
363+
return false;
364+
}
365+
if (argumentType.isEmptySequence()) {
366+
return false;
367+
}
368+
ItemType argumentItemType = argumentType.getItemType();
369+
if (!argumentItemType.isAtomicItemType()) {
370+
return false;
371+
}
372+
if (BuiltinTypesCatalogue.untypedAtomicItem.isSubtypeOf(argumentItemType)) {
373+
return false;
374+
}
375+
return argumentType.isSubtypeOf(expectedType);
376+
}
357377

358378
@Override
359379
public String toString() {

‎src/main/java/org/rumbledb/context/VariableValues.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,17 @@ public void addVariableCount(Name varName, Item count) {
179179
}
180180

181181
public List<Item> getLocalVariableValue(Name varName, ExceptionMetadata metadata) {
182-
if (this.localVariableValues.containsKey(varName) && this.localVariableValues.get(varName) == null) {
182+
List<Item> localValue = this.localVariableValues.get(varName);
183+
if (localValue != null) {
184+
return localValue;
185+
}
186+
if (this.localVariableValues.containsKey(varName)) {
183187
// Referencing an uninitialized local variable is illegal
184188
throw new RumbleException(
185189
"Runtime error retrieving variable " + varName + " value",
186190
metadata
187191
);
188192
}
189-
if (this.localVariableValues.containsKey(varName)) {
190-
return this.localVariableValues.get(varName);
191-
}
192193

193194
if (this.rddVariableValues.containsKey(varName)) {
194195
if (this.nestedQuery) {

‎src/main/java/org/rumbledb/runtime/functions/io/UriCollectionFunctionIterator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ public UriCollectionFunctionIterator(
2323

2424
@Override
2525
public Item materializeFirstItemOrNull(DynamicContext context) {
26-
throw new UnimplementedFunctionException("fn:uri-collection", getMetadata());
26+
throw new UnimplementedFunctionException("uri-collection", getMetadata());
2727
}
2828
}

‎src/main/java/org/rumbledb/runtime/functions/strings/CodepointsToStringFunctionIterator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ private String buildStringFromCodepoints(RuntimeIterator argumentIterator, Strin
7777
}
7878

7979
private int extractCodePoint(Item item) {
80+
if (item.isInt()) {
81+
return item.getIntValue();
82+
}
8083
try {
8184
return item.getIntegerValue().intValueExact();
8285
} catch (ArithmeticException e) {

‎src/main/java/org/rumbledb/runtime/functions/strings/StringLengthFunctionIterator.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public StringLengthFunctionIterator(
5050
public Item materializeFirstItemOrNull(DynamicContext context) {
5151
if (this.getChildren().size() == 0) {
5252
List<Item> items = context.getVariableValues().getLocalVariableValue(Name.CONTEXT_ITEM, getMetadata());
53-
return ItemFactory.getInstance().createIntItem(items.get(0).getStringValue().length());
53+
return ItemFactory.getInstance().createIntItem(codePointLength(items.get(0).getStringValue()));
5454
}
5555
Item stringItem = this.getChild(0)
5656
.materializeFirstItemOrNull(context);
@@ -59,7 +59,11 @@ public Item materializeFirstItemOrNull(DynamicContext context) {
5959
return ItemFactory.getInstance().createIntItem(0);
6060
}
6161

62-
return ItemFactory.getInstance().createIntItem(stringItem.getStringValue().length());
62+
return ItemFactory.getInstance().createIntItem(codePointLength(stringItem.getStringValue()));
63+
}
64+
65+
private static int codePointLength(String value) {
66+
return value.codePointCount(0, value.length());
6367
}
6468

6569
@Override

‎src/main/java/org/rumbledb/runtime/functions/util/formatting/NumericFormattingSupport.java‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,36 @@ public static String applyGrouping(
3838
return digits;
3939
}
4040

41-
String working = groupFromRight ? new StringBuilder(digits).reverse().toString() : digits;
41+
int length = digits.length();
4242

4343
if (repeatingInterval != null) {
44-
// Repeating grouping uses a single separator across all positions.
45-
String separator = new String(Character.toChars(groupingPositions.get(0).separatorCP()));
46-
StringBuilder sb = new StringBuilder();
47-
for (int i = 0; i < working.length(); i++) {
48-
if (i > 0 && i % repeatingInterval == 0) {
49-
sb.append(separator);
44+
int interval = repeatingInterval;
45+
if (length <= interval) {
46+
return digits;
47+
}
48+
int separatorCP = groupingPositions.get(0).separatorCP();
49+
int separatorLength = Character.charCount(separatorCP);
50+
int separatorCount = (length - 1) / interval;
51+
StringBuilder sb = new StringBuilder(length + separatorCount * separatorLength);
52+
if (groupFromRight) {
53+
int firstGroup = ((length - 1) % interval) + 1;
54+
sb.append(digits, 0, firstGroup);
55+
for (int i = firstGroup; i < length; i += interval) {
56+
sb.appendCodePoint(separatorCP);
57+
sb.append(digits, i, i + interval);
58+
}
59+
} else {
60+
sb.append(digits, 0, interval);
61+
for (int i = interval; i < length; i += interval) {
62+
sb.appendCodePoint(separatorCP);
63+
sb.append(digits, i, Math.min(i + interval, length));
5064
}
51-
sb.append(working.charAt(i));
5265
}
53-
return groupFromRight ? sb.reverse().toString() : sb.toString();
66+
return sb.toString();
5467
}
5568

69+
String working = groupFromRight ? new StringBuilder(digits).reverse().toString() : digits;
70+
5671
List<GroupingPos> gps = new ArrayList<>(groupingPositions);
5772
gps.sort(Comparator.comparingInt(GroupingPos::distanceFromAnchor));
5873

‎src/main/java/org/rumbledb/runtime/functions/util/formatting/language/LanguageSupport.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ public final class LanguageSupport {
1616
private LanguageSupport() {
1717
}
1818

19+
/** Normalizes, applies the ICU-support fallback, and resolves to a ULocale, cached by language string. */
20+
public static ULocale resolveEffectiveULocale(String language) {
21+
return ULOCALE_CACHE.computeIfAbsent(
22+
language == null ? DEFAULT_LANGUAGE : language,
23+
l -> ULocale.forLanguageTag(effectiveLanguageOf(normalizeLanguage(l)))
24+
);
25+
}
26+
1927
public static String normalizeLanguage(String language) {
2028
if (language == null || language.trim().isEmpty()) {
2129
return DEFAULT_LANGUAGE;
@@ -35,9 +43,4 @@ public static String effectiveLanguageOf(String normalizedLanguage) {
3543
public static Locale resolveLocale(String language) {
3644
return Locale.forLanguageTag(normalizeLanguage(language));
3745
}
38-
39-
/** Returns the cached ULocale for an already-effective (normalized, ICU-supported) language string. */
40-
public static ULocale resolveULocale(String effectiveLanguage) {
41-
return ULOCALE_CACHE.computeIfAbsent(effectiveLanguage, ULocale::forLanguageTag);
42-
}
4346
}

‎src/main/java/org/rumbledb/runtime/functions/util/formatting/pictures/FormatInteger/IntegerPictureFormatter.java‎

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package org.rumbledb.runtime.functions.util.formatting.pictures.FormatInteger;
22

3-
import com.ibm.icu.util.ULocale;
4-
53
import org.rumbledb.exceptions.ExceptionMetadata;
64
import org.rumbledb.runtime.functions.util.formatting.NumberWords;
75
import org.rumbledb.runtime.functions.util.formatting.NumericFormattingSupport;
@@ -58,52 +56,34 @@ public static String format(BigInteger value, String pictureString, String langu
5856
boolean isNegative = value.signum() < 0;
5957
BigInteger absValue = value.abs();
6058

61-
// Resolve the locale once and pass it to the handlers.
62-
ULocale locale = LanguageSupport.resolveULocale(
63-
LanguageSupport.effectiveLanguageOf(LanguageSupport.normalizeLanguage(language))
59+
FormatIntegerPicture picture = PICTURE_CACHE.computeIfAbsent(
60+
pictureString,
61+
key -> FormatIntegerPictureParser.parse(key, metadata)
6462
);
65-
66-
FormatIntegerPicture picture = PICTURE_CACHE.get(pictureString);
67-
if (picture == null) {
68-
picture = FormatIntegerPictureParser.parse(pictureString, metadata);
69-
PICTURE_CACHE.putIfAbsent(pictureString, picture);
70-
}
7163
PrimaryFormatToken primary = picture.getPrimaryFormatToken();
7264
IntegerFormatModifier modifier = picture.getFormatModifier();
7365

74-
String result;
66+
String result = switch (primary.getType()) {
67+
case PrimaryFormatToken.DECIMAL -> handleDecimal(absValue, primary, modifier, language);
68+
case PrimaryFormatToken.ALPHABETIC_UPPER, PrimaryFormatToken.ALPHABETIC_LOWER ->
69+
handleAlphabetic(absValue, primary, modifier, language);
70+
case PrimaryFormatToken.ROMAN_UPPER, PrimaryFormatToken.ROMAN_LOWER ->
71+
handleRoman(absValue, primary, modifier, language);
72+
case PrimaryFormatToken.WORDS_LOWER, PrimaryFormatToken.WORDS_UPPER, PrimaryFormatToken.WORDS_TITLE ->
73+
handleWords(absValue, primary, modifier, language);
74+
default -> handleOther(absValue, modifier, language);
75+
};
7576

7677
// Invariant: value => 0
7778

78-
switch (primary.getType()) {
79-
case PrimaryFormatToken.DECIMAL:
80-
result = handleDecimal(absValue, primary, modifier, locale);
81-
break;
82-
case PrimaryFormatToken.ALPHABETIC_UPPER:
83-
case PrimaryFormatToken.ALPHABETIC_LOWER:
84-
result = handleAlphabetic(absValue, primary, modifier, locale);
85-
break;
86-
case PrimaryFormatToken.ROMAN_UPPER:
87-
case PrimaryFormatToken.ROMAN_LOWER:
88-
result = handleRoman(absValue, primary, modifier, locale);
89-
break;
90-
case PrimaryFormatToken.WORDS_LOWER:
91-
case PrimaryFormatToken.WORDS_UPPER:
92-
case PrimaryFormatToken.WORDS_TITLE:
93-
result = handleWords(absValue, primary, modifier, locale);
94-
break;
95-
default:
96-
result = handleOther(absValue, modifier, locale);
97-
}
98-
9979
return !isNegative ? result : ("-" + result);
10080
}
10181

10282
private static String handleDecimal(
10383
BigInteger value,
10484
PrimaryFormatToken primary,
10585
IntegerFormatModifier modifier,
106-
ULocale locale
86+
String language
10787
) {
10888
NumericPicture picture = primary.getNumericPicture();
10989

@@ -122,7 +102,7 @@ private static String handleDecimal(
122102
// now)
123103

124104
if (IntegerFormatModifier.ORDINAL.equals(modifier.getNumberType())) {
125-
digits = digits + NumberWords.ordinalSuffix(value, locale);
105+
digits = digits + NumberWords.ordinalSuffix(value, LanguageSupport.resolveEffectiveULocale(language));
126106
}
127107

128108
return digits;
@@ -132,10 +112,10 @@ private static String handleRoman(
132112
BigInteger value,
133113
PrimaryFormatToken primary,
134114
IntegerFormatModifier modifier,
135-
ULocale locale
115+
String language
136116
) {
137117
if (value.signum() == 0 || value.compareTo(BigInteger.valueOf(3999)) > 0) {
138-
return handleOther(value, modifier, locale);
118+
return handleOther(value, modifier, language);
139119
}
140120

141121
// For Roman, unsupported ordinal handling is ignored and cardinal numbering is used.
@@ -146,10 +126,10 @@ private static String handleAlphabetic(
146126
BigInteger value,
147127
PrimaryFormatToken primary,
148128
IntegerFormatModifier modifier,
149-
ULocale locale
129+
String language
150130
) {
151131
if (value.signum() == 0 || value.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) > 0) {
152-
return handleOther(value, modifier, locale);
132+
return handleOther(value, modifier, language);
153133
}
154134

155135
String result = NumericFormattingSupport.integerToAlphabetic(
@@ -165,18 +145,26 @@ private static String handleWords(
165145
BigInteger value,
166146
PrimaryFormatToken primary,
167147
IntegerFormatModifier modifier,
168-
ULocale locale
148+
String language
169149
) {
170150
if (value.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) > 0) {
171-
return handleOther(value, modifier, locale);
151+
return handleOther(value, modifier, language);
172152
}
173153

174154
String result;
175155

176156
if (IntegerFormatModifier.ORDINAL.equals(modifier.getNumberType())) {
177-
result = NumberWords.ordinalWords(value.longValueExact(), locale, modifier.getFormatSpecifier());
157+
result = NumberWords.ordinalWords(
158+
value.longValueExact(),
159+
LanguageSupport.resolveEffectiveULocale(language),
160+
modifier.getFormatSpecifier()
161+
);
178162
} else {
179-
result = NumberWords.cardinal(value.longValueExact(), locale, modifier.getFormatSpecifier());
163+
result = NumberWords.cardinal(
164+
value.longValueExact(),
165+
LanguageSupport.resolveEffectiveULocale(language),
166+
modifier.getFormatSpecifier()
167+
);
180168
}
181169

182170
if (primary.getType().equals(PrimaryFormatToken.WORDS_LOWER)) {
@@ -190,12 +178,12 @@ private static String handleWords(
190178
private static String handleOther(
191179
BigInteger value,
192180
IntegerFormatModifier modifier,
193-
ULocale locale
181+
String language
194182
) {
195183
String result = NumericFormattingSupport.toDecimalString(value);
196184

197185
if (IntegerFormatModifier.ORDINAL.equals(modifier.getNumberType())) {
198-
result = result + NumberWords.ordinalSuffix(value, locale);
186+
result = result + NumberWords.ordinalSuffix(value, LanguageSupport.resolveEffectiveULocale(language));
199187
}
200188

201189
return result;

0 commit comments

Comments
 (0)