Revert "Dynamically enable/disable packet coalecsing and test it"
This reverts commit 60ab7ef00ac0a988ee2672c636d946c964e6fa41.
diff --git a/include/grpc/grpc_cronet.h b/include/grpc/grpc_cronet.h
index 566c34a..295e0f5 100644
--- a/include/grpc/grpc_cronet.h
+++ b/include/grpc/grpc_cronet.h
@@ -44,8 +44,6 @@
void *engine, const char *target, const grpc_channel_args *args,
void *reserved);
-GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing);
-
#ifdef __cplusplus
}
#endif
diff --git a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c
index 2e40020..477cf07 100644
--- a/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c
+++ b/src/core/ext/transport/cronet/client/secure/cronet_channel_create.c
@@ -51,8 +51,6 @@
extern grpc_transport_vtable grpc_cronet_vtable;
-bool grpc_cronet_packet_coalescing_enabled = true;
-
GRPCAPI grpc_channel *grpc_cronet_secure_channel_create(
void *engine, const char *target, const grpc_channel_args *args,
void *reserved) {
@@ -69,7 +67,3 @@
return grpc_channel_create(&exec_ctx, target, args,
GRPC_CLIENT_DIRECT_CHANNEL, (grpc_transport *)ct);
}
-
-GRPCAPI void grpc_cronet_use_packet_coalescing(bool use_coalescing) {
- grpc_cronet_packet_coalescing_enabled = use_coalescing;
-}
diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c
index 5429eb3..447f3f3 100644
--- a/src/core/ext/transport/cronet/transport/cronet_transport.c
+++ b/src/core/ext/transport/cronet/transport/cronet_transport.c
@@ -61,8 +61,6 @@
/* TODO (makdharma): Hook up into the wider tracing mechanism */
int grpc_cronet_trace = 0;
-extern bool grpc_cronet_packet_coalescing_enabled;
-
enum e_op_result {
ACTION_TAKEN_WITH_CALLBACK,
ACTION_TAKEN_NO_CALLBACK,
@@ -152,13 +150,12 @@
bool state_callback_received[OP_NUM_OPS];
bool fail_state;
bool flush_read;
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
bool flush_cronet_when_ready;
bool pending_write_for_trailer;
+#endif
bool unprocessed_send_message;
grpc_error *cancel_error;
-
- /* Whether packet coalescing is enabled */
- bool packet_coalescing_enabled;
/* data structure for storing data coming from server */
struct read_state rs;
/* data structure for storing data going to the server */
@@ -428,10 +425,12 @@
}
/* Send the initial metadata on wire if there is no SEND_MESSAGE or
* SEND_TRAILING_METADATA ops pending */
- if (s->state.packet_coalescing_enabled && s->state.flush_cronet_when_ready) {
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ if (s->state.flush_cronet_when_ready) {
CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_flush (%p)", s->cbs);
bidirectional_stream_flush(stream);
}
+#endif
gpr_mu_unlock(&s->mu);
execute_from_storage(s);
}
@@ -569,10 +568,10 @@
CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs);
s->state.state_callback_received[OP_SEND_MESSAGE] = false;
bidirectional_stream_write(s->cbs, "", 0, true);
- if (s->state.packet_coalescing_enabled) {
- CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs);
- bidirectional_stream_flush(s->cbs);
- }
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs);
+ bidirectional_stream_flush(s->cbs);
+#endif
s->state.state_op_done[OP_SEND_TRAILING_METADATA] = true;
gpr_mu_unlock(&s->mu);
@@ -769,9 +768,11 @@
result = false;
/* we haven't got on_write_completed for the send yet */
else if (stream_state->state_op_done[OP_SEND_MESSAGE] &&
- !stream_state->state_callback_received[OP_SEND_MESSAGE] &&
- !(stream_state->packet_coalescing_enabled &&
- stream_state->pending_write_for_trailer))
+ !stream_state->state_callback_received[OP_SEND_MESSAGE]
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ && !stream_state->pending_write_for_trailer
+#endif
+ )
result = false;
} else if (op_id == OP_CANCEL_ERROR) {
/* already executed */
@@ -857,10 +858,10 @@
s->cbs = bidirectional_stream_create(s->curr_ct.engine, s->curr_gs,
&cronet_callbacks);
CRONET_LOG(GPR_DEBUG, "%p = bidirectional_stream_create()", s->cbs);
- if (stream_state->packet_coalescing_enabled) {
- bidirectional_stream_disable_auto_flush(s->cbs, true);
- bidirectional_stream_delay_request_headers_until_flush(s->cbs, true);
- }
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ bidirectional_stream_disable_auto_flush(s->cbs, true);
+ bidirectional_stream_delay_request_headers_until_flush(s->cbs, true);
+#endif
char *url = NULL;
const char *method = "POST";
s->header_array.headers = NULL;
@@ -871,10 +872,11 @@
CRONET_LOG(GPR_DEBUG, "bidirectional_stream_start(%p, %s)", s->cbs, url);
bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false);
stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true;
- if (stream_state->packet_coalescing_enabled && !stream_op->send_message &&
- !stream_op->send_trailing_metadata) {
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ if (!stream_op->send_message && !stream_op->send_trailing_metadata) {
s->state.flush_cronet_when_ready = true;
}
+#endif
result = ACTION_TAKEN_WITH_CALLBACK;
} else if (stream_op->send_message &&
op_can_be_run(stream_op, stream_state, &oas->state,
@@ -911,18 +913,19 @@
stream_state->state_callback_received[OP_SEND_MESSAGE] = false;
bidirectional_stream_write(s->cbs, stream_state->ws.write_buffer,
(int)write_buffer_size, false);
- if (stream_state->packet_coalescing_enabled) {
- if (!stream_op->send_trailing_metadata) {
- CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs);
- bidirectional_stream_flush(s->cbs);
- result = ACTION_TAKEN_WITH_CALLBACK;
- } else {
- stream_state->pending_write_for_trailer = true;
- result = ACTION_TAKEN_NO_CALLBACK;
- }
- } else {
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ if (!stream_op->send_trailing_metadata) {
+ CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)",
+ s->cbs);
+ bidirectional_stream_flush(s->cbs);
result = ACTION_TAKEN_WITH_CALLBACK;
+ } else {
+ stream_state->pending_write_for_trailer = true;
+ result = ACTION_TAKEN_NO_CALLBACK;
}
+#else
+ result = ACTION_TAKEN_WITH_CALLBACK;
+#endif
} else {
result = NO_ACTION_POSSIBLE;
}
@@ -941,10 +944,10 @@
s->cbs);
stream_state->state_callback_received[OP_SEND_MESSAGE] = false;
bidirectional_stream_write(s->cbs, "", 0, true);
- if (stream_state->packet_coalescing_enabled) {
- CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs);
- bidirectional_stream_flush(s->cbs);
- }
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
+ CRONET_LOG(GPR_DEBUG, "bidirectional_stream_flush (%p)", s->cbs);
+ bidirectional_stream_flush(s->cbs);
+#endif
result = ACTION_TAKEN_WITH_CALLBACK;
}
stream_state->state_op_done[OP_SEND_TRAILING_METADATA] = true;
@@ -1173,9 +1176,10 @@
sizeof(s->state.state_callback_received));
s->state.fail_state = s->state.flush_read = false;
s->state.cancel_error = NULL;
+#ifdef GRPC_CRONET_WITH_PACKET_COALESCING
s->state.flush_cronet_when_ready = s->state.pending_write_for_trailer = false;
+#endif
s->state.unprocessed_send_message = false;
- s->state.packet_coalescing_enabled = grpc_cronet_packet_coalescing_enabled;
gpr_mu_init(&s->mu);
return 0;
}
diff --git a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m
index 9bbf3cd..dcd7f2f 100644
--- a/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m
+++ b/src/objective-c/tests/CronetUnitTests/CronetUnitTests.m
@@ -269,9 +269,7 @@
grpc_completion_queue_destroy(cq);
}
-- (void)PacketCoalescing:(bool)use_coalescing {
- grpc_cronet_use_packet_coalescing(use_coalescing);
-
+- (void)testPacketCoalescing {
grpc_call *c;
grpc_slice request_payload_slice =
grpc_slice_from_copied_string("hello world");
@@ -381,7 +379,7 @@
long len;
bool coalesced = false;
while ((len = SSL_read(ssl, buf, sizeof(buf))) > 0) {
- gpr_log(GPR_DEBUG, "Read len: %ld", len);
+ NSLog(@"Read len: %ld", len);
// Analyze the HTTP/2 frames in the same TLS PDU to identify if
// coalescing is successful
@@ -406,7 +404,7 @@
}
}
- XCTAssert(coalesced == use_coalescing);
+ XCTAssert(coalesced);
SSL_free(ssl);
SSL_CTX_free(ctx);
close(s);
@@ -435,9 +433,4 @@
grpc_completion_queue_destroy(cq);
}
-- (void)testPacketCoalescing {
- [self PacketCoalescing:false];
- [self PacketCoalescing:true];
-}
-
@end
diff --git a/src/objective-c/tests/Podfile b/src/objective-c/tests/Podfile
index 3760330..462c6a8 100644
--- a/src/objective-c/tests/Podfile
+++ b/src/objective-c/tests/Podfile
@@ -97,6 +97,7 @@
# GPR_UNREACHABLE_CODE causes "Control may reach end of non-void
# function" warning
config.build_settings['GCC_WARN_ABOUT_RETURN_TYPE'] = 'NO'
+ config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] = '$(inherited) COCOAPODS=1 GRPC_CRONET_WITH_PACKET_COALESCING=1'
end
end