Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use string as hashcode in lock map in HttpUrlConnectorProvider #5815

Conversation

ripdajacker
Copy link

See issue: #5814

This patch changes the key type of the locks map in org.glassfish.jersey.client.HttpUrlConnectorProvider to avoid calling getInetAddress on the URL's every time a URL connection is opened.

This is an issue when using the built-in JVM http proxy mechanism.

@senivam
Copy link
Contributor

senivam commented Dec 3, 2024

please try to sign ECA (Wiki) to merge your PR. Otherwise, we cannot accept it. And thank you for the fix!

@ripdajacker ripdajacker changed the title Use string as hashcode in lock map in httpurlconnectorprovider Use string as hashcode in lock map in HttpUrlConnectorProvider Dec 3, 2024
@ripdajacker
Copy link
Author

ripdajacker commented Dec 3, 2024

I've signed the ECA :-)

@jansupol
Copy link
Contributor

jansupol commented Dec 4, 2024

@ripdajacker Thank you for this.
However, #5813 drops the lock at all and would be even better performance-wise.

@ripdajacker
Copy link
Author

@ripdajacker Thank you for this. However, #5813 drops the lock at all and would be even better performance-wise.

That seems to be for the 2.1 branch.

My fix does not fix the concurrency implications of using locks but it removes the very subtle potential DNS lookup when attempting to require a lock.

I am certain there are better ways to improve performance, but cannot see how the linked PR fixes the issue at hand.

@jansupol
Copy link
Contributor

jansupol commented Dec 4, 2024

@ripdajacker Sorry a wrong link, midnight work... #5794 should be the fix.

@ripdajacker
Copy link
Author

That will probably do, will it be merged to 3.1 too?

@senivam
Copy link
Contributor

senivam commented Dec 5, 2024

@ripdajacker yes, before every release of 3.0.x and 3.1.x changes from 2.x and 3.0 branches are propagated respectively.

@ripdajacker
Copy link
Author

@senivam Cool, good to know :) Is there an ETA?

I'll close the PR then.

@ripdajacker ripdajacker closed this Dec 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jersey update from 3.1.8 to 3.1.9 performance regression when using the JVM http.proxyHost mechanism
3 participants