Commit Graph

63 Commits

Author SHA1 Message Date
Craig Tiller bbea27de46
Reland HPACK parsing changes (#26997)
* Revert "Revert "HPACK Table --> C++ (#26851)" (#26995)"

This reverts commit 840bcce9c4.

* fix bad parsing of trailing === in binary metadata
2021-08-13 10:57:42 -07:00
Craig Tiller 840bcce9c4
Revert "HPACK Table --> C++ (#26851)" (#26995)
This reverts commit 83bcb0cf2e.
2021-08-12 11:20:19 -07:00
Craig Tiller 83bcb0cf2e
HPACK Table --> C++ (#26851)
* Buffer HPACK parsing until the end of a header boundary

HTTP2 headers are sent in (potentially) many frames, but all must be
sent sequentially with no traffic intervening.

This was not clear when I wrote the HPACK parser, and still indeed quite
contentious on the HTTP2 mailing lists.

Now that matter is well settled (years ago!) take advantage of the fact
by delaying parsing until all bytes are available.

A future change will leverage this to avoid having to store and verify
partial parse state, completely eliminating indirect calls within the
parser.

* maybe fixes

* xx

* fix boundary detection

* clang-format

* Revert "xx"

This reverts commit 258d712ed3.

* fix tests

* add missed check

* fixes

* fix

* update tests

* fix benchmark

* properly unref

* optimize final slice refcounting

* cleanup bm_chttp2_hpack

* start

* new parser progress

* refinement

* get it compiling

* bug-fix

* build files

* clang-tidy

* fixes

* fixes

* fixes

* fix-leaks

* clang-tidy

* comments

* fix merge error

* Revert "Buffer HPACK parsing until the end of a header boundary (#26700)"

This reverts commit 8bab3e4bf4.

* streaming hpack parser start

* streaming parser

* clang-format

* Rework HPackTable into C++

* clang-tidy

* fix merge

* actually set the size of the entries array

* better
2021-08-09 16:07:41 -07:00
Craig Tiller 613b90ba93
Revert "Buffer HPACK parsing until the end of a header boundary (#26700)" (#26825)
This reverts commit 8bab3e4bf4.
2021-07-29 11:22:47 -07:00
Craig Tiller 8bab3e4bf4
Buffer HPACK parsing until the end of a header boundary (#26700)
HTTP2 headers are sent in (potentially) many frames, but all must be
sent sequentially with no traffic intervening.

This was not clear when I wrote the HPACK parser, and still indeed quite
contentious on the HTTP2 mailing lists.

Now that matter is well settled (years ago!) take advantage of the fact
by delaying parsing until all bytes are available.

A future change will leverage this to avoid having to store and verify
partial parse state, completely eliminating indirect calls within the
parser.
2021-07-28 15:36:51 -07:00
Craig Tiller d3e5803cb2
Move HPACK parser to C++ (#26689)
This is a fairly low effort migration of the current codebase into a C++ class, instead of free standing C code.
It builds upon #26657 as a necessary first step.
I've tried to minimize any changes to semantics or logic in this change, except where required to get a minimal amount of encapsulation - which is the major aim of this change.

A future change in this series will buffer slices until all HPACK headers are in memory for a stream prior to decoding -- it's important to have an encapsulated API to the parser before doing so however (hence this CL).

The next change after that will be an almost complete rewrite of the parsing functionality -- since we'll have the total set of header bytes, we'll no longer need to support suspending decoding at arbitrary points. This will allow us to move to a simple recursive descent parser, eliminate a bunch of indirection in this code, and end up in a much more malleable place for when we start doing metadata API changes.

(we likely also end up with some good performance wins!)
2021-07-15 13:51:23 -07:00
Esun Kim ca945a58e9
Introduced grpc_error_handle (#25902)
- Define grpc_error_handle
- Replace grpc_error* with grpc_error_handle
2021-04-22 12:00:28 -07:00
Esun Kim 62ac3f075a Added call to grpc::testing::TestEnvironment in tests 2020-07-01 11:37:57 -07:00
Esun Kim 3a519a0b64 Replaced grpc_core New & Delete with C++ new & delete 2019-11-06 17:52:26 -08:00
Vijay Pai 406b70629a Remove unused parameter warning (17 of 20) 2019-10-17 08:52:00 -07:00
Arjun Roy 0b06676c9e hpack encoder optimizations.
Removed some cycles and branches from hpack_enc for CH2.
Specifically:
1. Pushed certain metadata key/value length checks to
prepare_application_metadata() in src/core/lib/surface/call.cc.
This means that rather than check all key/val lengths for all metadata, we only
do so for custom added user metadata. Inside CH2, we change the length checks to
debug checks so we can catch if core/filter metadata fails to pass the check.

2. Changed various asserts to debug asserts when able.

3. Refactored some of the header emission code to remove duplicated code.

4. Un-inlined some logging methods.

This results in somewhat faster hpack_encoder performance:

BM_HpackEncoderInitDestroy
222ns ± 0%              221ns ± 0%   -0.29%        (p=0.000 n=34+34)
BM_HpackEncoderEncodeDeadline
[framing_bytes/iter:9 header_bytes/iter:6       ]               135ns ± 1%
124ns ± 0%   -8.05%        (p=0.000 n=39+38)
BM_HpackEncoderEncodeHeader<EmptyBatch>/0/16384
[framing_bytes/iter:9 header_bytes/iter:0       ]              34.2ns ± 0%
34.2ns ± 0%   -0.01%        (p=0.014 n=34+38)
BM_HpackEncoderEncodeHeader<EmptyBatch>/1/16384
[framing_bytes/iter:9 header_bytes/iter:0       ]              34.2ns ± 0%
34.2ns ± 0%   -0.04%        (p=0.004 n=34+37)
BM_HpackEncoderEncodeHeader<SingleStaticElem>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.5ns ± 0%
45.9ns ± 0%   -3.28%        (p=0.000 n=28+38)
BM_HpackEncoderEncodeHeader<SingleInternedKeyElem>/0/16384
[framing_bytes/iter:9 header_bytes/iter:6       ]              77.0ns ± 1%
68.3ns ± 1%  -11.33%        (p=0.000 n=39+40)
BM_HpackEncoderEncodeHeader<SingleInternedElem>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.7ns ± 1%
45.5ns ± 0%   -4.63%        (p=0.000 n=39+33)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<1, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.2ns ± 0%
45.3ns ± 0%   -3.96%        (p=0.000 n=33+34)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<3, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.7ns ± 0%
45.6ns ± 0%   -4.54%        (p=0.000 n=38+40)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<10, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.7ns ± 0%
45.5ns ± 0%   -4.63%        (p=0.000 n=39+32)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<31, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.8ns ± 0%
45.6ns ± 1%   -4.59%        (p=0.000 n=38+39)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<100, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.8ns ± 0%
45.5ns ± 0%   -4.64%        (p=0.000 n=39+36)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<1, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.3ns ± 0%
45.3ns ± 0%   -4.09%        (p=0.000 n=38+36)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<3, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.8ns ± 1%
45.6ns ± 0%   -4.71%        (p=0.000 n=37+40)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<10, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.7ns ± 0%
45.5ns ± 0%   -4.66%        (p=0.000 n=39+32)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<31, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.8ns ± 1%
45.6ns ± 1%   -4.62%        (p=0.000 n=37+39)
BM_HpackEncoderEncodeHeader<SingleInternedBinaryElem<100, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.7ns ± 0%
45.5ns ± 0%   -4.67%        (p=0.000 n=38+32)
BM_HpackEncoderEncodeHeader<SingleNonInternedElem>/0/16384
[framing_bytes/iter:9 header_bytes/iter:9       ]              80.5ns ± 1%
74.7ns ± 0%   -7.16%        (p=0.000 n=38+35)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<1, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:12      ]               105ns ± 1%
99ns ± 0%   -5.91%        (p=0.000 n=38+34)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<3, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:14      ]               111ns ± 1%
106ns ± 1%   -4.86%         (p=0.020 n=39+2)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<10, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:23      ]               135ns ± 0%
130ns ± 0%   -3.45%         (p=0.020 n=35+2)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<31, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:46      ]               225ns ± 1%
223ns ± 0%   -0.91%         (p=0.003 n=37+2)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<100, false>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:120     ]               467ns ± 0%
472ns ± 0%   +1.09%         (p=0.003 n=38+2)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<1, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:12      ]              81.6ns ± 1%
74.8ns ± 0%   -8.40%        (p=0.000 n=37+33)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<3, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:14      ]              82.0ns ± 1%
74.8ns ± 0%   -8.80%        (p=0.000 n=37+32)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<10, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:21      ]              82.1ns ± 1%
74.9ns ± 0%   -8.86%        (p=0.000 n=35+34)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<31, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:42      ]              97.6ns ± 2%
91.8ns ± 0%   -5.95%        (p=0.000 n=35+27)
BM_HpackEncoderEncodeHeader<SingleNonInternedBinaryElem<100, true>>/0/16384
[framing_bytes/iter:9 header_bytes/iter:111     ]              97.2ns ± 1%
91.2ns ± 2%   -6.19%        (p=0.000 n=37+38)
BM_HpackEncoderEncodeHeader<SingleNonInternedElem>/0/1
[framing_bytes/iter:54 header_bytes/iter:9      ]               230ns ± 0%
221ns ± 0%   -3.91%        (p=0.000 n=38+37)
BM_HpackEncoderEncodeHeader<MoreRepresentativeClientInitialMetadata>/0/16384
[framing_bytes/iter:9 header_bytes/iter:16      ]               206ns ± 2%
170ns ± 1%  -17.51%        (p=0.000 n=39+39)
BM_HpackEncoderEncodeHeader<RepresentativeServerInitialMetadata>/0/16384
[framing_bytes/iter:9 header_bytes/iter:3       ]              66.4ns ± 2%
62.5ns ± 1%   -5.85%        (p=0.000 n=34+39)
BM_HpackEncoderEncodeHeader<RepresentativeServerTrailingMetadata>/1/16384
[framing_bytes/iter:9 header_bytes/iter:1       ]              47.5ns ± 0%
45.9ns ± 1%   -3.29%        (p=0.000 n=26+38)
2019-09-06 13:43:29 -07:00
Vijay Pai 9bb1247bdb
Merge pull request #16327 from vjpai/keep_running
C++ microbenchmarks: update from KeepRunning to range-based for
2019-09-05 11:07:53 -07:00
Vijay Pai 1077b3435c Use range-based for on state rather than state.KeepRunning when possible 2019-08-29 11:27:00 -07:00
Arjun Roy b46e3668d3 s/branch/tail_call/ for CH2 on_hdr().
on_hdr() checks if a void-return function pointer is null before jumping to it.
If it is null, it returns an error; else it executes that function and returns
success.

This change converts the void-returning function to one that returns a
grpc_error* and thus saves a branch in on_hdr() (since we're branching once by
following the function pointer anyways, we're effectively coalescing these two
branches).
2019-08-28 13:06:11 -07:00
Hope Casey-Allen 59564ebd96 Fix warnings to unblock gcc8 support 2019-08-20 16:02:53 -07:00
Arjun Roy b1d73a01f1 Removed duplicate static table from hpack table. Removed an or instruction for
every usage of static grpc metadata. Inlined hpack table lookups for static
metadata.

This leads to faster hpack parser creation:
BM_HpackParserInitDestroy 5.32µs ± 1% 0.06µs ± 1%  -98.91% (p=0.000 n=18+19)

And slightly faster parsing:
BM_HpackParserParseHeader<RepresentativeClientInitialMetadata, OnInitialHeader>
456ns ± 1%              435ns ± 1%   -4.74%        (p=0.000 n=18+19)
BM_HpackParserParseHeader<MoreRepresentativeClientInitialMetadata,
OnInitialHeader>
1.06µs ± 2%             1.04µs ± 2%   -1.82%        (p=0.000 n=19+20)

It also yields a slight (0.5 - 1.0 microsecond) reduction in CPU time for
fullstack unary pingpong:
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/0/512
[polls/iter:3.0001                         ]            23.9µs ± 2%
23.0µs ± 1%  -3.63%          (p=0.002 n=6+6)
BM_UnaryPingPong<TCP, NoOpMutator, NoOpMutator>/0/32768
[polls/iter:3.00015                        ]            35.1µs ± 1%
34.2µs ± 1%  -2.57%          (p=0.036 n=5+3)
BM_UnaryPingPong<MinTCP, NoOpMutator, NoOpMutator>/8/0
[polls/iter:3.00011                        ]            21.7µs ± 3%
21.2µs ± 2%  -2.44%          (p=0.017 n=6+5)
2019-07-15 15:39:31 -07:00
Esun Kim e18ed03c04 Made gRPC inialized after entering main function in microbenchmarks. 2019-05-02 16:58:37 -07:00
Arjun Roy 8ce42f67b2 Shrink arena size by 40 bytes and add additional
alignment options (for cache-alignment).

We shrink by:
1) Removing an unnecessary zone pointer.
2) Replacing gpr_mu (40 bytes when using pthread_mutex_t) with
std::atomic_flag.

We also header-inline the fastpath alloc (ie. when not doing a zone
alloc) and move the malloc() for a zone alloc outside of the mutex
critical zone, which allows us to replace the mutex with a spinlock.

We also cache-align created arenas.
2019-04-24 16:39:03 -07:00
Soheil Hassas Yeganeh 48e4a81b05 Remeve memset(0) from arena allocated memory.
Callers are updated to properly initialize the memory.

This behavior can be overridden using GRPC_ARENA_INIT_STRATEGY
environment variable.
2018-11-05 10:12:39 -05:00
Hope Casey-Allen 4c6e7ce15d Destroy metadata buffer at end of benchmark loop 2018-09-20 12:56:56 -07:00
Hope Casey-Allen d44feec92f Reassign arena pointer instead of stomping on memory 2018-09-20 09:54:59 -07:00
Hope Casey-Allen 4b721fbde0 Destroy arena at end of benchmark to not leak memory 2018-09-19 20:51:45 -07:00
Hope Casey-Allen 29d9489ea9 Increase initial arena size to be more representative of real workload scenario and increase frequency of recreating the arena to avoid oom 2018-09-19 20:46:17 -07:00
Hope Casey-Allen 91727bd015 Move arena create outside of benchmark, format, and typo fix 2018-09-19 19:13:23 -07:00
Hope Casey-Allen 967bbcd5d3 Fixing benchmark name and adding a new one 2018-09-13 18:38:01 -07:00
Noah Eisen 58e0cbf9fb Enable the performance-* clang-tidy checks 2018-06-14 14:58:09 -04:00
ncteisen 40ec89ff67 Support microbenchmarks internally 2018-04-25 17:41:23 -07:00
Vijay Pai 2f4161c210 Use stack frame size limits for consistency with internal builds 2018-03-05 16:43:32 -08:00
Noah Eisen 4d20a66685 Run clang fmt 2018-02-09 09:34:04 -08:00
Noah Eisen be82e64b3d Autofix c casts to c++ casts 2018-02-09 09:16:55 -08:00
Yash Tibrewal 8cf1470a51
Revert "Revert "All instances of exec_ctx being passed around in src/core removed"" 2017-12-06 09:47:54 -08:00
Yash Tibrewal ad4d2dde00
Revert "All instances of exec_ctx being passed around in src/core removed" 2017-12-06 09:05:05 -08:00
Yash Tibrewal c354269ba7 Remove _ prefixed variable names 2017-11-29 19:07:44 -08:00
Yash Tibrewal 177039b2f8 Merge master 2017-11-16 19:31:28 -08:00
Yash Tibrewal 6c26b16fe0 Move ExecCtx to grpc_core namespace. Make exec_ctx a private static in ExecCtx and some minor changes 2017-11-14 18:11:22 -08:00
Yash Tibrewal 75122c2357 Address some PR comments 2017-11-14 01:36:28 -08:00
Craig Tiller 4ac2b8e585 Enable clang-tidy as a sanity check, fix up all known failures 2017-11-10 14:14:17 -08:00
Yash Tibrewal 0032548674 Correction to closure.cc,bm_chttp2_hpack and few more 2017-11-10 10:48:15 -08:00
Yash Tibrewal 3150744c71 Removing more exec_ctx instances 2017-11-09 18:27:37 -08:00
Yash Tibrewal 4e9265c828 Merge with master 2017-11-09 17:46:29 -08:00
Yash Tibrewal 40422d5fa6 Merge master 2017-11-06 14:43:31 -08:00
Craig Tiller baa14a975e Update clang-format to 5.0 2017-11-03 09:09:36 -07:00
Yash Tibrewal 39aed1ae8b Remove unnecessary extern Cs 2017-10-25 17:04:35 -07:00
ncteisen c296e82e11 clang fmt 2017-10-24 08:58:14 -07:00
ncteisen 9e3eedb6af Remove old header benchmark 2017-10-24 08:33:00 -07:00
ncteisen 6bf4bcef04 Fix bm_diff 2017-10-23 22:19:01 -07:00
yang-g 83085aa74f Add a microbm, seeing 195ns with current impl and 162ns with new impl 2017-10-19 16:11:06 -07:00
Yash Tibrewal 0ee7574732 Removing instances of exec_ctx being passed around in functions in
src/core. exec_ctx is now a thread_local pointer of type ExecCtx instead of
grpc_exec_ctx which is initialized whenever ExecCtx is instantiated. ExecCtx
also keeps track of the previous exec_ctx so that nesting of exec_ctx is
allowed. This means that there is only one exec_ctx being used at any
time. Also, grpc_exec_ctx_finish is called in the destructor of the
object, and the previous exec_ctx is restored to avoid breaking current
functionality. The code still explicitly calls grpc_exec_ctx_finish
because removing all such instances causes the code to break.
2017-10-18 17:12:19 -07:00
yang-g c010d1d18a Update benchmark according to new encoding method 2017-10-16 17:57:46 -07:00
yang-g c94c7cc5b5 restore existing fixtures 2017-10-16 17:57:46 -07:00