Skip to content

Commit

Permalink
Add short circuiting returns to equals method in OptionsBase.
Browse files Browse the repository at this point in the history
asMap() is expensive as it reflectively aggregates all fields into a map. We can return quickly if they are the same instance or instances of differing classes. If we do get to comparing different instances of the same class, we can return as soon as we find a differing option field.

RELNOTES: None.
PiperOrigin-RevId: 244687761
  • Loading branch information
Googler authored and copybara-github committed Apr 22, 2019
1 parent 30306d5 commit deca59b
Showing 1 changed file with 15 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.escape.Escaper;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
* Base class for all options classes. Extend this class, adding public instance fields annotated
Expand Down Expand Up @@ -119,8 +120,21 @@ public static String mapToCacheKey(Map<String, Object> optionsMap) {
}

@Override
@SuppressWarnings("EqualsGetClass") // Options can only be equal if they are of the same type.
public final boolean equals(Object that) {
return that instanceof OptionsBase && this.asMap().equals(((OptionsBase) that).asMap());
if (this == that) {
return true;
}
if (that == null || !getClass().equals(that.getClass())) {
return false;
}
OptionsBase other = (OptionsBase) that;
for (OptionDefinition def : OptionsParser.getOptionDefinitions(getClass())) {
if (!Objects.equals(getValueFromDefinition(def), other.getValueFromDefinition(def))) {
return false;
}
}
return true;
}

@Override
Expand Down

0 comments on commit deca59b

Please sign in to comment.