Skip to content

8372460: Use EnumMap instead of HashMap for DateTimeFormatter parsing to improve performance #28936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RogerRiggs wants to merge 1 commit into from
+32 −5
Closed

8372460: Use EnumMap instead of HashMap for DateTimeFormatter parsing to improve performance #28936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/time/format/DateTimeParseContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ TemporalAccessor toResolved(ResolverStyle resolverStyle, Set<TemporalField> reso
* @return the value mapped to the specified field, null if field was not parsed
*/
Long getParsed(TemporalField field) {
return currentParsed().fieldValues.get(field);
return currentParsed().getFieldValue(field);
}

/**
Expand All @@ -362,7 +362,7 @@ Long getParsed(TemporalField field) {
*/
int setParsedField(TemporalField field, long value, int errorPos, int successPos) {
Objects.requireNonNull(field, "field");
Long old = currentParsed().fieldValues.put(field, value);
Long old = currentParsed().putFieldValue(field, value);
return (old != null && old.longValue() != value) ? ~errorPos : successPos;
}

Expand Down
33 changes: 30 additions & 3 deletions src/java.base/share/classes/java/time/format/Parsed.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@
import java.time.Period;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.chrono.ChronoLocalDate;
import java.time.chrono.ChronoLocalDateTime;
import java.time.chrono.ChronoZonedDateTime;
Expand All @@ -98,6 +97,7 @@
import java.time.temporal.TemporalQueries;
import java.time.temporal.TemporalQuery;
import java.time.temporal.UnsupportedTemporalTypeException;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
Expand Down Expand Up @@ -128,7 +128,7 @@ final class Parsed implements TemporalAccessor {
/**
* The parsed fields.
*/
final Map<TemporalField, Long> fieldValues = new HashMap<>();
private Map<TemporalField, Long> fieldValues = initFieldValuesMap();
/**
* The parsed zone.
*/
Expand Down Expand Up @@ -178,6 +178,10 @@ final class Parsed implements TemporalAccessor {
Parsed copy() {
// only copy fields used in parsing stage
Parsed cloned = new Parsed();
if (!(fieldValues instanceof EnumMap)) {
// Upgrade to a full TemporalField Map
cloned.fieldValues = new HashMap<>(this.fieldValues.size());
}
cloned.fieldValues.putAll(this.fieldValues);
cloned.zone = this.zone;
cloned.zoneNameType = this.zoneNameType;
Expand All @@ -187,6 +191,29 @@ Parsed copy() {
return cloned;
}


// A bit contorted way to get the map created.
@SuppressWarnings("unchecked")
private Map<TemporalField, Long> initFieldValuesMap() {
Copy link
Member

@liach liach Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private Map<TemporalField, Long> initFieldValuesMap() {
private static Map<TemporalField, Long> initFieldValuesMap() {

Let's make this static to be early-construction safe.

return (Map<TemporalField, Long>) (Object)new EnumMap<ChronoField, Long>(ChronoField.class);
}

Long putFieldValue(TemporalField field, long value) {
try {
return fieldValues.put(field, value);
} catch (ClassCastException cce) {
// Upgrade to a full TemporalField Map
Map<TemporalField, Long> nmap = new HashMap<>(fieldValues.size() + 1);
Copy link
Member

@liach liach Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Map<TemporalField, Long> nmap = new HashMap<>(fieldValues.size() + 1);
Map<TemporalField, Long> nmap = HashMap.newHashMap(fieldValues.size() + 1);

We might still anticipate more elements to be put into the map, so a better alternative might be just new HashMap<>(fieldValues)?

nmap.putAll(fieldValues);
fieldValues = nmap;
return nmap.put(field, value);
}
}

Long getFieldValue(TemporalField field) {
return fieldValues.get(field);
}

//-----------------------------------------------------------------------
@Override
public boolean isSupported(TemporalField field) {
Expand Down Expand Up @@ -334,7 +361,7 @@ private void resolveFields() {
}

private void updateCheckConflict(TemporalField targetField, TemporalField changeField, Long changeValue) {
Long old = fieldValues.put(changeField, changeValue);
Long old = putFieldValue(changeField, changeValue);
if (old != null && old.longValue() != changeValue.longValue()) {
throw new DateTimeException("Conflict found: " + changeField + " " + old +
" differs from " + changeField + " " + changeValue +
Expand Down