From ffdcad5e0699f36b5b1a45aa5a8a8e2733d592f1 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 24 Aug 2018 16:07:49 -0700 Subject: Redefine constants from errqueue.h. Some header files lag behind the kernel version --- src/core/lib/iomgr/tcp_posix.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 1db2790265..ac1e919acb 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -646,7 +646,8 @@ struct cmsghdr* process_timestamp(grpc_tcp* tcp, msghdr* msg, return cmsg; } - auto tss = reinterpret_cast(CMSG_DATA(cmsg)); + auto tss = + reinterpret_cast(CMSG_DATA(cmsg)); auto serr = reinterpret_cast(CMSG_DATA(next_cmsg)); if (serr->ee_errno != ENOMSG || serr->ee_origin != SO_EE_ORIGIN_TIMESTAMPING) { -- cgit v1.2.3 From c33b593c8863ccc496e913bd82744ecc991c89f8 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Thu, 27 Sep 2018 14:23:29 -0700 Subject: Add comments on what 'covering' a write means --- src/core/lib/iomgr/tcp_posix.cc | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index ac1e919acb..4aad83915f 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -204,6 +204,15 @@ static void drop_uncovered(grpc_tcp* tcp) { GPR_ASSERT(old_count != 1); } +/* gRPC API considers a Write operation to be done the moment it clears ‘flow + control’ i.e., and not necessarily sent on the wire. This means that the + application MAY NOT call `grpc_completion_queue_next/pluck` in a timely + manner when its `Write()` API is acked. + + We need to ensure that the fd is 'covered' (i.e being monitored by some + polling thread and progress is made) and hence add it to a backup poller + here */ + static void cover_self(grpc_tcp* tcp) { backup_poller* p; gpr_atm old_count = -- cgit v1.2.3 From f443f185b3dd4b694e9719d7e730e98396416ce8 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Fri, 28 Sep 2018 19:48:38 -0700 Subject: Address feedback comment --- src/core/lib/iomgr/tcp_posix.cc | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 4aad83915f..e40bf81c90 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -204,15 +204,13 @@ static void drop_uncovered(grpc_tcp* tcp) { GPR_ASSERT(old_count != 1); } -/* gRPC API considers a Write operation to be done the moment it clears ‘flow - control’ i.e., and not necessarily sent on the wire. This means that the - application MAY NOT call `grpc_completion_queue_next/pluck` in a timely - manner when its `Write()` API is acked. - - We need to ensure that the fd is 'covered' (i.e being monitored by some - polling thread and progress is made) and hence add it to a backup poller - here */ - +// gRPC API considers a Write operation to be done the moment it clears ‘flow +// control’ i.e., not necessarily sent on the wire. This means that the +// application MIGHT not call `grpc_completion_queue_next/pluck` in a timely +// manner when its `Write()` API is acked. +// +// We need to ensure that the fd is 'covered' (i.e being monitored by some +// polling thread and progress is made) and hence add it to a backup poller here static void cover_self(grpc_tcp* tcp) { backup_poller* p; gpr_atm old_count = -- cgit v1.2.3 From c8d5db17173318ff5efe3e0721bca3340251f738 Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Thu, 11 Oct 2018 15:27:27 -0400 Subject: Update TCP read estimates as soon as we read the whole buffer. If we have a continous stream of bytes on the socket, we will never grow the buffer, because we will never get EAGAIN, and call finish. This is a serious performance issue, which can be misued. As soon as we have a full buffer, update the estimate. --- src/core/lib/iomgr/tcp_posix.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index e40bf81c90..7fd0e91346 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -468,7 +468,9 @@ static void tcp_do_read(grpc_tcp* tcp) { GRPC_STATS_INC_TCP_READ_SIZE(read_bytes); add_to_estimate(tcp, static_cast(read_bytes)); GPR_ASSERT((size_t)read_bytes <= tcp->incoming_buffer->length); - if (static_cast(read_bytes) < tcp->incoming_buffer->length) { + if (static_cast(read_bytes) == tcp->incoming_buffer->length) { + finish_estimate(tcp); + } else if (static_cast(read_bytes) < tcp->incoming_buffer->length) { grpc_slice_buffer_trim_end( tcp->incoming_buffer, tcp->incoming_buffer->length - static_cast(read_bytes), -- cgit v1.2.3 From 78434ad30360583ab4f71c1aca1dc3e850a31e98 Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Thu, 11 Oct 2018 15:28:49 -0400 Subject: Do not wait for allocation if buffer is less than half the target. We overallocate by 2x for target. Unless buffer is more than half full, we should not delay read for more allocation. --- src/core/lib/iomgr/tcp_posix.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core/lib/iomgr/tcp_posix.cc') diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 7fd0e91346..aa2704ce26 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -500,7 +500,7 @@ static void tcp_read_allocation_done(void* tcpp, grpc_error* error) { static void tcp_continue_read(grpc_tcp* tcp) { size_t target_read_size = get_target_read_size(tcp); - if (tcp->incoming_buffer->length < target_read_size && + if (tcp->incoming_buffer->length < target_read_size / 2 && tcp->incoming_buffer->count < MAX_READ_IOVEC) { if (grpc_tcp_trace.enabled()) { gpr_log(GPR_INFO, "TCP:%p alloc_slices", tcp); -- cgit v1.2.3