Try to eager load Servlets at startup. #23970
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will trigger all the required CDI events, with as side-effect that we also created the Servlet in advance. According to the Servlet spec, this is allowed, and the Servlet TCK indeed passes with this change. Signed-off-by: Arjan Tijms <arjan.tijms@gmail.com>
pzygielo
reviewed
Jun 6, 2022
| continue; | ||
| nonLoadOnStartupServlets.add(wrapper); | ||
| } else { | ||
| loadOnStartupServlets.computeIfAbsent(loadOnStartup, e -> new ArrayList<>()) |
There was a problem hiding this comment.
Suggested change
| loadOnStartupServlets.computeIfAbsent(loadOnStartup, e -> new ArrayList<>()) | |
| loadOnStartupServlets.computeIfAbsent(loadOnStartup, ArrayList::new) |
There was a problem hiding this comment.
I think arraylist::new will not work, as the key will be input into the ctor. With the key being an integer and the ctor setting the size, they will cause an out of memory with the huge key values being used.
There was a problem hiding this comment.
Wow, Arjan is right, I had the same idea, we discussed it in a chat and I started googling:
https://community.sonarsource.com/t/java-bug-using-collectors-tomap-with-arraylist-new/7661
There was a problem hiding this comment.
I'm happy to learn it now and not from the GF bug 😄
dmatej
approved these changes
Jun 6, 2022
| for (Wrapper wrapper : nonLoadOnStartupServlets) { | ||
| try { | ||
| wrapper.tryLoad(); | ||
| } catch (Throwable t) { |
There was a problem hiding this comment.
One day I will remove all these dangerous "catch-all" blocks. ;-)
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will trigger all the required CDI events, with as side-effect that
we also created the Servlet in advance.
According to the Servlet spec, this is allowed, and the Servlet TCK
indeed passes with this change.
Signed-off-by: Arjan Tijms arjan.tijms@gmail.com