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

Issue 795573002: Implement zero-copy SSL buffers. (Closed)

Created:
6 years ago by haavardm
Modified:
6 years ago
Reviewers:
agl, Ryan Sleevi
CC:
chromium-reviews, cbentzel+watch_chromium.org, davidben
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement zero-copy SSL buffers. Use the new BoringSSL zero copy interface to avoid allocating new buffers for each read and write operation and save buffer memory. BUG=169885 Committed: https://crrev.com/2d92e72b7dbde513405cc79dd648360daec8c2c1 Cr-Commit-Position: refs/heads/master@{#309191}

Patch Set 1 #

Total comments: 26

Patch Set 2 : Rebase #

Patch Set 3 : Address comments #

Patch Set 4 : Fixed CHECK failure caused by removing setting offset to 0 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -30 lines) Patch
M net/socket/ssl_client_socket_openssl.h View 1 chunk +4 lines, -2 lines 0 comments Download
M net/socket/ssl_client_socket_openssl.cc View 1 2 3 7 chunks +50 lines, -28 lines 0 comments Download

Messages

Total messages: 21 (5 generated)
haavardm
A new go for implementing zero-copy buffers using a new BoringSSL api. Ryan: Please review ...
6 years ago (2014-12-10 11:18:24 UTC) #2
Ryan Sleevi
On 2014/12/10 11:18:24, haavardm wrote: > A new go for implementing zero-copy buffers using a ...
6 years ago (2014-12-11 00:48:22 UTC) #3
haavardm
On 2014/12/11 00:48:22, Ryan Sleevi wrote: > On 2014/12/10 11:18:24, haavardm wrote: > > A ...
6 years ago (2014-12-17 08:52:17 UTC) #4
Ryan Sleevi
David's out of office (per the updated name) for the holidays, redirecting to Adam. Who ...
6 years ago (2014-12-17 10:47:56 UTC) #6
agl
https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc File net/socket/ssl_client_socket_openssl.cc (right): https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc#newcode71 net/socket/ssl_client_socket_openssl.cc:71: // Default sizes of the internal BoringSSL buffers. // ...
6 years ago (2014-12-17 19:12:22 UTC) #7
haavardm
https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.h File net/socket/ssl_client_socket_openssl.h (right): https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.h#newcode192 net/socket/ssl_client_socket_openssl.h:192: scoped_refptr<GrowableIOBuffer> send_buffer_; On 2014/12/17 19:12:22, agl wrote: > I'm ...
6 years ago (2014-12-17 22:04:53 UTC) #8
agl
LGTM with nits. https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc File net/socket/ssl_client_socket_openssl.cc (right): https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc#newcode1531 net/socket/ssl_client_socket_openssl.cc:1531: DCHECK(status == 1); // Should never ...
6 years ago (2014-12-17 22:29:57 UTC) #9
Ryan Sleevi
lgtm https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc File net/socket/ssl_client_socket_openssl.cc (right): https://codereview.chromium.org/795573002/diff/1/net/socket/ssl_client_socket_openssl.cc#newcode1571 net/socket/ssl_client_socket_openssl.cc:1571: // traffic, this over-subscribed Read()ing will not cause ...
6 years ago (2014-12-17 22:57:10 UTC) #10
haavardm
PTAL Note that I have not adjusted the default buffer size, as I think that ...
6 years ago (2014-12-18 12:18:08 UTC) #11
agl
lgtm
6 years ago (2014-12-18 19:16:50 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/795573002/40001
6 years ago (2014-12-18 19:18:21 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tests_recipe/builds/39172)
6 years ago (2014-12-18 22:05:05 UTC) #16
haavardm
On 2014/12/18 22:05:05, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
6 years ago (2014-12-18 22:33:49 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/795573002/60001
6 years ago (2014-12-19 13:34:26 UTC) #19
commit-bot: I haz the power
Committed patchset #4 (id:60001)
6 years ago (2014-12-19 13:46:10 UTC) #20
commit-bot: I haz the power
6 years ago (2014-12-19 13:47:15 UTC) #21
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/2d92e72b7dbde513405cc79dd648360daec8c2c1
Cr-Commit-Position: refs/heads/master@{#309191}

Powered by Google App Engine
This is Rietveld 408576698