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

Avoid using whenComplete(); use handle() #1596

Merged
merged 1 commit into from
Feb 15, 2019

Conversation

trustin
Copy link
Member

@trustin trustin commented Feb 15, 2019

Motivation:

As we discovered in #1440, whenComplete() is harmful for performance.

Modifications:

  • Use handle() instead of whenComplete()

Result:

  • Potentially less unnecessary instantiation of CompletionException.

Motivation:

As we discovered in line#1440, `whenComplete()` is harmful for performance.

Modifications:

- Use `handle()` instead of `whenComplete()`

Result:

- Potentially less unnecessary instantiation of `CompletionException`.
@trustin trustin added this to the 0.80.0 milestone Feb 15, 2019
@trustin trustin removed the request for review from hyangtack February 15, 2019 06:50
@codecov
Copy link

codecov bot commented Feb 15, 2019

Codecov Report

Merging #1596 into master will increase coverage by 0.02%.
The diff coverage is 80%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1596      +/-   ##
============================================
+ Coverage     72.84%   72.87%   +0.02%     
+ Complexity     7576     7575       -1     
============================================
  Files           699      699              
  Lines         30459    30459              
  Branches       3712     3712              
============================================
+ Hits          22188    22197       +9     
+ Misses         6362     6353       -9     
  Partials       1909     1909
Impacted Files Coverage Δ Complexity Δ
.../web/reactive/ArmeriaReactiveWebServerFactory.java 57.89% <100%> (ø) 28 <0> (ø) ⬇️
.../reactive/ArmeriaHttpClientResponseSubscriber.java 61.97% <66.66%> (ø) 12 <0> (ø) ⬇️
.../linecorp/armeria/client/retrofit2/PipeBuffer.java 79.06% <0%> (-4.66%) 6% <0%> (ø)
...corp/armeria/common/stream/FixedStreamMessage.java 85.71% <0%> (-3.58%) 20% <0%> (-2%)
.../armeria/client/endpoint/dns/DnsEndpointGroup.java 82.97% <0%> (-1.07%) 13% <0%> (-1%)
...common/stream/AbstractStreamMessageDuplicator.java 76.79% <0%> (+0.49%) 9% <0%> (ø) ⬇️
...rp/armeria/common/stream/DefaultStreamMessage.java 91.42% <0%> (+1.42%) 54% <0%> (+1%) ⬆️
...om/linecorp/armeria/client/HttpSessionHandler.java 61.2% <0%> (+1.72%) 29% <0%> (+1%) ⬆️
...armeria/client/HttpClientPipelineConfigurator.java 73.06% <0%> (+2.69%) 36% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b95b992...d75b3f9. Read the comment docs.

@trustin trustin merged commit 2475fcc into line:master Feb 15, 2019
@trustin trustin deleted the avoid_when_complete branch February 15, 2019 07:42
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this pull request Sep 19, 2020
Motivation:

As we discovered in line#1440, `whenComplete()` is harmful for performance.

Modifications:

- Use `handle()` instead of `whenComplete()`

Result:

- Potentially less unnecessary instantiation of `CompletionException`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants