Increase HPACK decoder dynamic table size limit. |
||||||||
Issue descriptionServers might want to encode headers with a dynamic table larger than the default 4 kB in order to reduce table churn and thus increase compression ratio. In order to allow for this, Chromium should have a decoder dynamic table size limit larger than 4 kB, and should communicate this limit to the server as a SETTINGS_HEADER_TABLE_SIZE value in the initial SETTINGS frame. Note that this has no memory usage implications on Chromium unless the server actually sends a dynamic table size update HPACK instruction. See also https://lists.w3.org/Archives/Public/ietf-http-wg/2016JulSep/0499.html for a public discussion on what the behavior should be.
,
Aug 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9ba3be0a5057b52aa35756a3fe3156301890097 commit d9ba3be0a5057b52aa35756a3fe3156301890097 Author: mdjones <mdjones@chromium.org> Date: Wed Aug 31 20:12:05 2016 Revert of Increase maximum size of the HPACK decoder dynamic table to 64 kB. (patchset #1 id:1 of https://codereview.chromium.org/2300683002/ ) Reason for revert: Suspected patch causing Android Cronet Builder and other bots to fail BidirectionalStreamTest#testSimpleGet. Will re-land if not. Original issue's description: > Increase maximum size of the HPACK decoder dynamic table to 64 kB. > > * Send out SETTINGS_HEADER_TABLE_SIZE = 64 kB in the initial SETTINGS frame on > each HTTP/2 connection. > * Immediately notify HpackDecoder about the change so that it allows the encoder > to update the dynamic table size up to this limit. It is safe to do so before > receiving the SETTINGS ACK, because the new limit is larger than the default 4 > kB. In fact, a server following RFC 7540 Section 6.5.3 word-by-word might > already use the larger limit before sending an ACK. > > BUG= 642784 > > Committed: https://crrev.com/30537e17941018ecffe781c009b39c7a8c4c8f30 > Cr-Commit-Position: refs/heads/master@{#415718} TBR=rch@chromium.org,bnc@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 642784 Review-Url: https://codereview.chromium.org/2302573002 Cr-Commit-Position: refs/heads/master@{#415735} [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/buffered_spdy_framer.cc [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/buffered_spdy_framer.h [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/hpack/hpack_decoder.h [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_framer.cc [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_framer.h [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_network_transaction_unittest.cc [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_session.cc [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_session.h [modify] https://crrev.com/d9ba3be0a5057b52aa35756a3fe3156301890097/net/spdy/spdy_session_unittest.cc
,
Sep 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 commit b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 Author: bnc <bnc@chromium.org> Date: Fri Sep 02 16:59:56 2016 Increase maximum size of the HPACK decoder dynamic table to 64 kB. * Send out SETTINGS_HEADER_TABLE_SIZE = 64 kB in the initial SETTINGS frame on each HTTP/2 connection. * Immediately notify HpackDecoder about the change so that it allows the encoder to update the dynamic table size up to this limit. It is safe to do so before receiving the SETTINGS ACK, because the new limit is larger than the default 4 kB. In fact, a server following RFC 7540 Section 6.5.3 word-by-word might already use the larger limit before sending an ACK. BUG= 642784 Committed: https://crrev.com/30537e17941018ecffe781c009b39c7a8c4c8f30 Review-Url: https://codereview.chromium.org/2300683002 Cr-Original-Commit-Position: refs/heads/master@{#415718} Cr-Commit-Position: refs/heads/master@{#416284} [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/buffered_spdy_framer.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/buffered_spdy_framer.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/hpack/hpack_decoder.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_framer.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_framer.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_network_transaction_unittest.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session_unittest.cc
,
Sep 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 commit b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 Author: bnc <bnc@chromium.org> Date: Fri Sep 02 16:59:56 2016 Increase maximum size of the HPACK decoder dynamic table to 64 kB. * Send out SETTINGS_HEADER_TABLE_SIZE = 64 kB in the initial SETTINGS frame on each HTTP/2 connection. * Immediately notify HpackDecoder about the change so that it allows the encoder to update the dynamic table size up to this limit. It is safe to do so before receiving the SETTINGS ACK, because the new limit is larger than the default 4 kB. In fact, a server following RFC 7540 Section 6.5.3 word-by-word might already use the larger limit before sending an ACK. BUG= 642784 Committed: https://crrev.com/30537e17941018ecffe781c009b39c7a8c4c8f30 Review-Url: https://codereview.chromium.org/2300683002 Cr-Original-Commit-Position: refs/heads/master@{#415718} Cr-Commit-Position: refs/heads/master@{#416284} [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/buffered_spdy_framer.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/buffered_spdy_framer.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/hpack/hpack_decoder.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_framer.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_framer.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_network_transaction_unittest.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session.cc [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session.h [modify] https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4/net/spdy/spdy_session_unittest.cc
,
Sep 13 2016
Requesting merge to M54 (2840). CL has landed 11 days ago, no related crashes or interop issues seen.
,
Sep 13 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13d05e85b2565345c84d071c83ede52eefa60c81 commit 13d05e85b2565345c84d071c83ede52eefa60c81 Author: Bence Béky <bnc@chromium.org> Date: Tue Sep 13 13:44:23 2016 Increase maximum size of the HPACK decoder dynamic table to 64 kB. * Send out SETTINGS_HEADER_TABLE_SIZE = 64 kB in the initial SETTINGS frame on each HTTP/2 connection. * Immediately notify HpackDecoder about the change so that it allows the encoder to update the dynamic table size up to this limit. It is safe to do so before receiving the SETTINGS ACK, because the new limit is larger than the default 4 kB. In fact, a server following RFC 7540 Section 6.5.3 word-by-word might already use the larger limit before sending an ACK. BUG= 642784 Committed: https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 Review-Url: https://codereview.chromium.org/2300683002 Cr-Original-Commit-Position: refs/heads/master@{#415718} Cr-Commit-Position: refs/heads/master@{#416284} (cherry picked from commit b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4) Review URL: https://codereview.chromium.org/2339463002 . Cr-Commit-Position: refs/branch-heads/2840@{#326} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/buffered_spdy_framer.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/buffered_spdy_framer.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/hpack/hpack_decoder.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_framer.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_framer.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_network_transaction_unittest.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session_unittest.cc
,
Sep 13 2016
,
Sep 26 2016
firefox made a very similar change in nightly 52 - and we received bug reports that it broke tomcat 8.5.. we've confirmed that canary is also broken with tomcat 8.5. If you've got anybody to reach out to wrt tomcat, that would be helpful. https://bugzilla.mozilla.org/show_bug.cgi?id=1305321
,
Oct 6 2016
Thanks patrick
,
Oct 6 2016
Assigning to ckrasic for now due to concerns about tomcat in #9
,
Oct 19 2016
Note Tomcat fix "in the following branches: 9.0.x for 9.0.0.M11 onwards, 8.5.x for 8.5.6 onwards", see https://bz.apache.org/bugzilla/show_bug.cgi?id=60173#c5. Also see issue 656953 .
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13d05e85b2565345c84d071c83ede52eefa60c81 commit 13d05e85b2565345c84d071c83ede52eefa60c81 Author: Bence Béky <bnc@chromium.org> Date: Tue Sep 13 13:44:23 2016 Increase maximum size of the HPACK decoder dynamic table to 64 kB. * Send out SETTINGS_HEADER_TABLE_SIZE = 64 kB in the initial SETTINGS frame on each HTTP/2 connection. * Immediately notify HpackDecoder about the change so that it allows the encoder to update the dynamic table size up to this limit. It is safe to do so before receiving the SETTINGS ACK, because the new limit is larger than the default 4 kB. In fact, a server following RFC 7540 Section 6.5.3 word-by-word might already use the larger limit before sending an ACK. BUG= 642784 Committed: https://crrev.com/b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4 Review-Url: https://codereview.chromium.org/2300683002 Cr-Original-Commit-Position: refs/heads/master@{#415718} Cr-Commit-Position: refs/heads/master@{#416284} (cherry picked from commit b3cafaaea4c3c0c1cf553d8ec9999f83a8c1c7f4) Review URL: https://codereview.chromium.org/2339463002 . Cr-Commit-Position: refs/branch-heads/2840@{#326} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/components/cronet/android/test/javatests/src/org/chromium/net/BidirectionalStreamTest.java [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/buffered_spdy_framer.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/buffered_spdy_framer.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/hpack/hpack_decoder.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_framer.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_framer.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_network_transaction_unittest.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session.cc [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session.h [modify] https://crrev.com/13d05e85b2565345c84d071c83ede52eefa60c81/net/spdy/spdy_session_unittest.cc
,
Feb 24 2017
Closing issue, because Tomcat bug has been fixed, and I haven't heard user complaints in a long while. See also links from comment #12. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bugdroid1@chromium.org
, Aug 31 2016