New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 690915 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 524258
issue 698403



Sign in to add a comment

Add a new Read method to Socket classes

Project Member Reported by xunji...@chromium.org, Feb 10 2017

Issue description

[split from  Issue 524258 ]
Add a new Read method to socket classes. The new read method 
allows caller to not retain an IOBuffer if desired. 

The design doc is at:
https://docs.google.com/document/d/1DsVcQ4-jsaIhu_OvIid-UHMUQfFqWYfmThMnQoZ1csc/edit#


 
Blocking: 698403
This will be done in 3 steps:
(1) TCP socket on posix:  https://codereview.chromium.org/2593063003/
(2) TCP socket on windows: https://codereview.chromium.org/2690943004/
(3) Make SpdySession use ReadIfReady: https://codereview.chromium.org/2734933002/
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/321a96f376e57651a1ad987f5585c0f07128e63a

commit 321a96f376e57651a1ad987f5585c0f07128e63a
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Mar 07 19:42:17 2017

Add Socket::ReadIfReady()

This CL adds a new read method to socket class. The new read method
allows caller to not retain an IOBuffer if desired.

The new functionality is hidden behind a field trial flag:
SocketReadIfReady.

The design doc is at:
https://docs.google.com/document/d/1DsVcQ4-jsaIhu_OvIid-UHMUQfFqWYfmThMnQoZ1csc/edit#

BUG= 690915 

Review-Url: https://codereview.chromium.org/2593063003
Cr-Commit-Position: refs/heads/master@{#455177}

[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/BUILD.gn
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/base/net_error_list.h
[add] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_bio_adapter.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_bio_adapter.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_bio_adapter_unittest.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_posix.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_posix.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_test_util.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/socket_test_util.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_client_socket.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_client_socket.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_socket_posix.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_socket_posix.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_socket_win.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/socket/tcp_socket_win.h
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/spdy/spdy_session.cc
[modify] https://crrev.com/321a96f376e57651a1ad987f5585c0f07128e63a/net/spdy/spdy_session_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18aeb05270bcd6fac5a33ebb8f1e3614662ac7ec

commit 18aeb05270bcd6fac5a33ebb8f1e3614662ac7ec
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Mar 07 22:06:55 2017

Add ReadIfReady() for tcp_socket_win

This CL adds ReadIfReady() support for TCP socket on Windows.
The new feature is hidden behind a field trial flag, SocketReadIfReady.
Design doc is linked in the bug below.

BUG= 690915 

Review-Url: https://codereview.chromium.org/2690943004
Cr-Commit-Position: refs/heads/master@{#455239}

[modify] https://crrev.com/18aeb05270bcd6fac5a33ebb8f1e3614662ac7ec/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/18aeb05270bcd6fac5a33ebb8f1e3614662ac7ec/net/socket/tcp_socket_win.cc
[modify] https://crrev.com/18aeb05270bcd6fac5a33ebb8f1e3614662ac7ec/net/socket/tcp_socket_win.h

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c182846fcda260cc3d11dbac775b1b7a666729c8

commit c182846fcda260cc3d11dbac775b1b7a666729c8
Author: xunjieli <xunjieli@chromium.org>
Date: Thu Mar 09 16:49:19 2017

Release SpdySession socket read buffer

This CL makes the following changes:

(1) Make SpdySession use Socket::ReadIfReady(). This new behavior is
    hidden behind a field trial flag, SocketReadIfReady.
(2) Make SpdySession lazily allocate |read_buffer| on demand.
(3) Make SpdySession release |read_buffer| when Read() completes or
    when ReadIfReady() cannot be completed synchronously.

Re-allocating an 8KiB buffer should be cheap, see more discussion
at https://groups.google.com/a/chromium.org/d/msg/project-trim/QtliUsApxyk/AOSnyuY7CgAJ

BUG= 690915 

Review-Url: https://codereview.chromium.org/2734933002
Cr-Commit-Position: refs/heads/master@{#455770}

[modify] https://crrev.com/c182846fcda260cc3d11dbac775b1b7a666729c8/net/spdy/spdy_session.cc
[modify] https://crrev.com/c182846fcda260cc3d11dbac775b1b7a666729c8/net/spdy/spdy_session.h

Status: Fixed (was: Started)

Sign in to add a comment