Optimize configuration value casts (#3688)#3805
Conversation
| return cast(value); | ||
| } | ||
|
|
||
| if (defaultValue instanceof Boolean) { |
There was a problem hiding this comment.
nit: Would it make sense to turn this if-else if to switch expression, something like this:
return switch (defaultValue()) {
case Boolean ignored -> cast(Boolean.valueOf(String.valueOf(value)));
case Integer ignored -> cast(Integer.valueOf(String.valueOf(value)));
case Long ignored -> cast(Long.valueOf(String.valueOf(value)));
case Double ignored -> cast(Double.valueOf(String.valueOf(value)));
case Float ignored -> cast(Float.valueOf(String.valueOf(value)));
case List<?> ignored -> cast(new ArrayList<>((List<?>) value));
case null, default -> cast(value);
};
There was a problem hiding this comment.
@nandorKollar
Good suggestion, but I think this compiles with --release 17, and switch pattern matching requires Java 21+. So we probably need to keep the if-else. But let me know if you have some suggestion.
Thank you!!
There was a problem hiding this comment.
Ah okay, that makes sense. Do we plan to compile with Java 21 in the near future?
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @jbonofre I just noticed that we recently moved to Java 17 minimum version for the clients.
dimas-b
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @AryanPatel226 ! Changes LGTM 👍
6e306f5 to
a3986ed
Compare
Optimizes
tryCastto avoid unnecessary string conversions when configuration values are already the correct type.The method was converting every value to string and parsing it back, even when the value already matched the expected type (Boolean, Integer, Long, Double). This adds type checks before string conversion to eliminate unnecessary overhead.
Fixes #3688
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)