-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Optimize new ArrayList() allocations whenever possible
#10573
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.
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
Optimize new ArrayList() allocations whenever possible
#10573
Conversation
To avoid resizing at runtime, it is better to use `ArrayList` with a predefined size. In some cases it is possible from the context of the list we are creating * Fix typos in Javadocs of the affected classes according to IDE suggestions * Use diamond operators for modern Java in the affected classes * pattern matching expressions for `if` where IDE suggested * Use `switch` in one place instead of `if..else`. Again: good suggestion from IDE * Fix Nullability for the router hierarchy * Have to add "redundant" cast in the `XPathRouter` to make NullAway happy
|
Let me know if this disturbanly long for review! |
cppwfs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic!
Thank you for doing this.
Just some nitpicks.
|
|
||
| private List<HandlerMethodArgumentResolver> buildArgumentResolvers(boolean listCapable) { | ||
| List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>(); | ||
| List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>(6); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is being a bit picky but I think a constant would be better here than the number 6.
| if (incomingCorrelationId != null) { | ||
| if (incomingSequenceDetails == null) { | ||
| incomingSequenceDetails = new ArrayList<>(); | ||
| incomingSequenceDetails = new ArrayList<>(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a constant here?
| validate(); | ||
|
|
||
| final List<FileListFilter<File>> filtersNeeded = new ArrayList<>(); | ||
| final List<FileListFilter<File>> filtersNeeded = new ArrayList<>(3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as well :-)
| public abstract class HttpRequestHandlingEndpointSupport extends BaseHttpInboundEndpoint { | ||
|
|
||
| private final List<HttpMessageConverter<?>> defaultMessageConverters = new ArrayList<>(); | ||
| private final List<HttpMessageConverter<?>> defaultMessageConverters = new ArrayList<>(10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
| public void afterPropertiesSet() { | ||
| List<Object> converters = new ArrayList<>(); | ||
| List<Object> converters = | ||
| new ArrayList<>(7 + (this.customConverters != null ? this.customConverters.length : 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use constant instead of 7.
I know Picky picky :-)
|
Any reasons asking for those constants? |
|
Why 6? I read the code and I understand why. But is there a constant we could use to add clarity? |
|
Again: those lists are local variables. Sorry, to say that but I wish you just bite this bullet and let the "magic number" a shot! Any other reasons not just clarity? |
cppwfs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you for the optimizations. This is great!
To avoid resizing at runtime, it is better to use
ArrayListwith a predefined size. In some cases it is possible from the context of the list we are creatingifwhere IDE suggestedswitchin one place instead ofif..else. Again: good suggestion from IDEXPathRouterto make NullAway happy