-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Make minor improvements to various RESTEasy Reactive classes that iterate through maps #18102
Conversation
…rate through maps We remove the use of entrySet() where it makes sense and replace it with forEach which faster and doesn't allocate additional objects
Something @Sanne would likely enjoy :) |
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 love that you're looking at such optimisation opportunities :) alas it's not complete.
@@ -185,7 +185,7 @@ public AbstractResponseBuilder clone() { | |||
responseBuilder.status = status; | |||
responseBuilder.reasonPhrase = reasonPhrase; | |||
responseBuilder.entity = entity; | |||
responseBuilder.metadata = new QuarkusMultivaluedHashMap<>(); | |||
responseBuilder.metadata = new CaseInsensitiveMap<>(); |
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.
How does this relate with the change?
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.
It's necessary in order to narrow the type of the map - it should have been done anyway
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.
Also see my comment below
...tive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/jaxrs/ResponseImpl.java
Show resolved
Hide resolved
It's great to be able to learn of such opportunities from the experts :) |
return stringHeaders; | ||
} | ||
|
||
public void populateStringHeaders(String headerName, List<Object> values) { | ||
List<String> stringValues = new ArrayList<>(values.size()); |
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 it's not related to this PR, but allocating arrayslists in a tight loop for no other reason than to accomodate for the put
method requirements ?! :)
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'm open to suggestions :)
We remove the use of
entrySet()
where it makes sense and replace it withforEach()
which faster and doesn't allocate additional objects