Skip to content

responseContent() remove excess ByteBufFlux conversion#2792

Merged
violetagg merged 6 commits intoreactor:mainfrom
manzhizhen:feat-responseContent-opt
May 5, 2023
Merged

responseContent() remove excess ByteBufFlux conversion#2792
violetagg merged 6 commits intoreactor:mainfrom
manzhizhen:feat-responseContent-opt

Conversation

@manzhizhen
Copy link
Copy Markdown
Contributor

@manzhizhen manzhizhen commented May 3, 2023

When I looked at the implementation of the class HttpClientFinalizer code, I found that the logic of the responseContent method was a bit strange. HttpClientFinalizer.contentReceiver represents ChannelOperations::receive, while ChannelOperations::receive internal implementation has already made a call to ByteBufFlux.fromInbound to return a ByteBufFlux, and a call and encapsulation of ByteBufFlux.fromInbound has also been made in the responseContent method, This is actually unnecessary, so you can directly refer to the internal implementation of ChannelOperations::receive in the responseContent method to generate ByteBufFlux, which will result in better performance and clearer logic. Similarly, there are similar issues with WebsocketFinalizer:responseContent, which is why this PR.

@manzhizhen manzhizhen closed this May 3, 2023
@manzhizhen manzhizhen reopened this May 3, 2023
@violetagg
Copy link
Copy Markdown
Member

@manzhizhen Please use ./gradlew spotlessApply in order to fix the copyright end date to 2023

@manzhizhen
Copy link
Copy Markdown
Contributor Author

@manzhizhen Please use ./gradlew spotlessApply in order to fix the copyright end date to 2023

ok, I do it.

@violetagg violetagg added the type/enhancement A general enhancement label May 3, 2023
@violetagg violetagg added this to the 1.0.32 milestone May 3, 2023
@violetagg
Copy link
Copy Markdown
Member

The failed test on CI for Windows OS is not relevant as it is in quic module, while this change is in http module.

@manzhizhen
Copy link
Copy Markdown
Contributor Author

The failed test on CI for Windows OS is not relevant as it is in quic module, while this change is in http module.

Okay, get it

Copy link
Copy Markdown
Contributor

@pderop pderop left a comment

Choose a reason for hiding this comment

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

looking good.

yizhenqiang added 2 commits May 4, 2023 22:15
…xcess ByteBufFlux conversion, preserve the contentReceiver constant
…xcess ByteBufFlux conversion, preserve the contentReceiver constant
@violetagg violetagg changed the base branch from main to 1.0.x May 5, 2023 06:21
@violetagg violetagg changed the base branch from 1.0.x to main May 5, 2023 06:22
@violetagg violetagg merged commit 162c11e into reactor:main May 5, 2023
@violetagg violetagg changed the title responseContent() remove excess ByteBufFlux conversion responseContent() remove excess ByteBufFlux conversion May 5, 2023
violetagg added a commit that referenced this pull request May 5, 2023
@violetagg
Copy link
Copy Markdown
Member

@manzhizhen Thanks for the PR.
I back ported the change to 1.0.x branch for 1.0.32 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants