Skip to content
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

No need to use response methods returning a Future, if not used (part II) #29502

Merged
merged 1 commit into from Dec 2, 2022

Conversation

franz1981
Copy link
Contributor

@franz1981 franz1981 commented Nov 25, 2022

Similarly to #25577 this is saving creating unnecessary garbage (that will live long enough to bother GC, affect performance and force Netty write completions advertising events we are not interested to deal with).

@franz1981
Copy link
Contributor Author

@geoand I would like to get the opinion of Vert-x team first, as per the mentioned PR.

@franz1981
Copy link
Contributor Author

@tsegismont Hi Thomas! Do you think this can cause problems similar to
#25577?

Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me but, as discussed elsewhere, this kind of optimization will no longer be effective when we upgrade to Netty 5 (void channel promise removal)

@franz1981
Copy link
Contributor Author

Thanks @tsegismont ! Yep we're aware, but we don't know when will happen and the existing users can have this small benefit in the meantime (tiny, but still something :) )

@geoand geoand marked this pull request as ready for review November 28, 2022 15:20
@geoand
Copy link
Contributor

geoand commented Nov 28, 2022

existing users can have this small benefit in the meantime (tiny, but still something :) )

every little bit helps :)

@geoand geoand merged commit 54a6694 into quarkusio:main Dec 2, 2022
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 2, 2022
@gsmet gsmet modified the milestones: 2.16 - main, 2.15.0.Final Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants