Skip to content

Revert "Immediately run write closures for failed stream"#16243

Merged
a11r merged 1 commit intogrpc:masterfrom
hcaseyal:revert_16179
Aug 6, 2018
Merged

Revert "Immediately run write closures for failed stream"#16243
a11r merged 1 commit intogrpc:masterfrom
hcaseyal:revert_16179

Conversation

@hcaseyal
Copy link
Copy Markdown
Contributor

@hcaseyal hcaseyal commented Aug 3, 2018

This PR caused some internal test failures. Reverting until we can find out why.

@hcaseyal hcaseyal requested a review from ncteisen August 3, 2018 20:32
@nicolasnoble nicolasnoble added the release notes: no Indicates if PR should not be in release notes label Aug 3, 2018
@ncteisen ncteisen mentioned this pull request Aug 3, 2018
@grpc-testing
Copy link
Copy Markdown

****************************************************************

libgrpc.so

     VM SIZE                                                                   FILE SIZE
 ++++++++++++++ GROWING                                                     ++++++++++++++

 -------------- SHRINKING                                                   --------------
  -0.1%     -16 src/core/ext/transport/chttp2/transport/chttp2_transport.cc     -16  -0.1%
      -2.5%     -16 grpc_chttp2_complete_closure_step                               -16  -2.5%

 -+-+-+-+-+-+-+ MIXED                                                       +-+-+-+-+-+-+-
  +0.0%     +16 [None]                                                          -32  -0.0%

  [ = ]       0 TOTAL                                                           -48  -0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]



@grpc-testing
Copy link
Copy Markdown

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

Objective-C binary sizes
*****************STATIC******************
  New size                      Old size
 1,980,675      Total (<)      1,980,683

 No significant differences in binary sizes

***************FRAMEWORKS****************
  New size                      Old size
10,811,769      Total (>)     10,811,767

 No significant differences in binary sizes


@srini100
Copy link
Copy Markdown
Contributor

srini100 commented Aug 3, 2018

Reverting this is going to reintroduce the bug in #15889. Also, this fix was made in conjunction with #15983 so who ever is going to work on this should look at both PRs.

@grpc-testing
Copy link
Copy Markdown

[microbenchmarks] No significant performance differences

@a11r a11r merged commit b85df07 into grpc:master Aug 6, 2018
@hcaseyal hcaseyal deleted the revert_16179 branch August 6, 2018 21:06
@lock lock bot locked as resolved and limited conversation to collaborators Nov 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

priority/P1 release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants