Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(537)

Issue 1992010: Revert 46757 - Fix IO thread hang on releasing a socket.... (Closed)

Created:
10 years, 7 months ago by sky
Modified:
9 years, 7 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Revert 46757 - Fix IO thread hang on releasing a socket. The problem was we assumed ProcessPendingRequest() would always make progress once the group's releasing socket went down to zero. However, in OnAvailableSocketSlot(), since the top stalled group might still have a releasing socket, it won't necessarily make progress. The algorithmic solution is to simply never do any socket slot processing in DoReleaseSocket() if there are any releasing sockets. This requires us to search for any stalled groups after releasing sockets gets back down to 0, so it requires the full group scan each time num_releasing_sockets goes back to 0. There is a performance hit here, but I think a linear search through 256~ groups in the worst case is ok. TODO(willchan): evaluate the perf hit and consider adding a secondary data structure to improve the stalled group search. BUG=42267 Review URL: http://codereview.chromium.org/2013009 TBR=willchan@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=46792

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -103 lines) Patch
M net/socket/client_socket_pool_base.h View 1 chunk +0 lines, -3 lines 0 comments Download
M net/socket/client_socket_pool_base.cc View 5 chunks +25 lines, -29 lines 0 comments Download
M net/socket/client_socket_pool_base_unittest.cc View 1 chunk +0 lines, -64 lines 0 comments Download
M net/socket/tcp_client_socket_pool_unittest.cc View 1 chunk +0 lines, -7 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
sky
10 years, 7 months ago (2010-05-08 20:17:48 UTC) #1
willchan no longer on Chromium
10 years, 7 months ago (2010-05-08 20:19:03 UTC) #2
If you haven't committed yet, you should explain the revert in the CL desc.

LGTM.

On Sat, May 8, 2010 at 1:17 PM,  <sky@chromium.org> wrote:
> Reviewers: willchan,
>
> Description:
> Revert 46757 - Fix IO thread hang on releasing a socket.
>
> The problem was we assumed ProcessPendingRequest() would always make
> progress
> once the group's releasing socket went down to zero.  However, in
> OnAvailableSocketSlot(), since the top stalled group might still have a
> releasing socket, it won't necessarily make progress.  The algorithmic
> solution
> is to simply never do any socket slot processing in DoReleaseSocket() if
> there
> are any releasing sockets.  This requires us to search for any stalled
> groups
> after releasing sockets gets back down to 0, so it requires the full group
> scan
> each time num_releasing_sockets goes back to 0.  There is a performance hit
> here, but I think a linear search through 256~ groups in the worst case is
> ok.
> TODO(willchan): evaluate the perf hit and consider adding a secondary data
> structure to improve the stalled group search.
> BUG=42267
>
> Review URL: http://codereview.chromium.org/2013009
>
> TBR=willchan@chromium.org
>
> Please review this at http://codereview.chromium.org/1992010/show
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     net/socket/client_socket_pool_base.h
>  M     net/socket/client_socket_pool_base.cc
>  M     net/socket/client_socket_pool_base_unittest.cc
>  M     net/socket/tcp_client_socket_pool_unittest.cc
>
>
>

Powered by Google App Engine
This is Rietveld 408576698