Skip to content

Commit 1c53314

Browse files
authored
Optimize zero copy of http body (#2915)
1 parent b6e4f03 commit 1c53314

3 files changed

Lines changed: 36 additions & 21 deletions

File tree

src/brpc/details/http_message.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ int HttpMessage::on_header_value(http_parser *parser,
141141
}
142142

143143
int HttpMessage::on_headers_complete(http_parser *parser) {
144-
HttpMessage *http_message = (HttpMessage *)parser->data;
144+
HttpMessage* http_message = (HttpMessage *)parser->data;
145145
http_message->_stage = HTTP_ON_HEADERS_COMPLETE;
146146
if (parser->http_major > 1) {
147147
// NOTE: this checking is a MUST because ProcessHttpResponse relies
@@ -282,9 +282,12 @@ int HttpMessage::OnBody(const char *at, const size_t length) {
282282
}
283283
if (!_read_body_progressively) {
284284
// Normal read.
285-
// TODO: The input data is from IOBuf as well, possible to append
286-
// data w/o copying.
287-
_body.append(at, length);
285+
if (NULL != _current_source_iobuf) {
286+
_current_source_iobuf->append_to(
287+
&_body, length, _parsed_block_size + (at - _current_block_base));
288+
} else {
289+
_body.append(at, length);
290+
}
288291
return 0;
289292
}
290293
// Progressive read.
@@ -434,13 +437,8 @@ const http_parser_settings g_parser_settings = {
434437

435438
HttpMessage::HttpMessage(bool read_body_progressively,
436439
HttpMethod request_method)
437-
: _parsed_length(0)
438-
, _stage(HTTP_ON_MESSAGE_BEGIN)
439-
, _request_method(request_method)
440-
, _read_body_progressively(read_body_progressively)
441-
, _body_reader(NULL)
442-
, _cur_value(NULL)
443-
, _vbodylen(0) {
440+
: _request_method(request_method)
441+
, _read_body_progressively(read_body_progressively) {
444442
http_parser_init(&_parser, HTTP_BOTH);
445443
_parser.allow_chunked_length = 1;
446444
_parser.data = this;
@@ -489,15 +487,23 @@ ssize_t HttpMessage::ParseFromIOBuf(const butil::IOBuf &buf) {
489487
<< ") to already-completed message";
490488
return -1;
491489
}
490+
_parsed_block_size = 0;
491+
_current_source_iobuf = &buf;
492+
BRPC_SCOPE_EXIT {
493+
_current_source_iobuf = NULL;
494+
};
492495
size_t nprocessed = 0;
493496
for (size_t i = 0; i < buf.backing_block_num(); ++i) {
494497
butil::StringPiece blk = buf.backing_block(i);
495498
if (blk.empty()) {
496499
// length=0 will be treated as EOF by http_parser, must skip.
497500
continue;
498501
}
499-
nprocessed += http_parser_execute(
502+
_current_block_base = blk.data();
503+
size_t n = http_parser_execute(
500504
&_parser, &g_parser_settings, blk.data(), blk.size());
505+
nprocessed += n;
506+
_parsed_block_size += n;
501507
if (_parser.http_errno != 0) {
502508
// May try HTTP on other formats, failure is norm.
503509
RPC_VLOG << "Fail to parse http message, parser=" << _parser

src/brpc/details/http_message.h

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,32 +99,38 @@ class HttpMessage {
9999
protected:
100100
int OnBody(const char* data, size_t size);
101101
int OnMessageComplete();
102-
size_t _parsed_length;
102+
size_t _parsed_length{0};
103103

104104
private:
105105
DISALLOW_COPY_AND_ASSIGN(HttpMessage);
106106
int UnlockAndFlushToBodyReader(std::unique_lock<butil::Mutex>& locked);
107107

108-
HttpParserStage _stage;
108+
HttpParserStage _stage{HTTP_ON_MESSAGE_BEGIN};
109109
std::string _url;
110-
HttpMethod _request_method;
110+
HttpMethod _request_method{HTTP_METHOD_GET};
111111
HttpHeader _header;
112-
bool _read_body_progressively;
112+
bool _read_body_progressively{false};
113113
// For mutual exclusion between on_body and SetBodyReader.
114114
butil::Mutex _body_mutex;
115115
// Read body progressively
116-
ProgressiveReader* _body_reader;
116+
ProgressiveReader* _body_reader{NULL};
117117
butil::IOBuf _body;
118118

119+
// Store the IOBuf information in `ParseFromIOBuf'
120+
// for later zero-copy usage in `OnBody'.
121+
const butil::IOBuf* _current_source_iobuf{NULL};
122+
const char* _current_block_base{NULL};
123+
size_t _parsed_block_size{0};
124+
119125
// Parser related members
120126
struct http_parser _parser;
121127
std::string _cur_header;
122-
std::string *_cur_value;
128+
std::string *_cur_value{NULL};
123129

124130
protected:
125131
// Only valid when -http_verbose is on
126132
std::unique_ptr<butil::IOBufBuilder> _vmsgbuilder;
127-
size_t _vbodylen;
133+
size_t _vbodylen{0};
128134
};
129135

130136
std::ostream& operator<<(std::ostream& os, const http_parser& parser);

test/brpc_http_message_unittest.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,15 +224,18 @@ TEST(HttpMessageTest, parse_from_iobuf) {
224224
"Content-Length: %lu\r\n"
225225
"\r\n",
226226
content_length);
227-
std::string content;
228-
for (size_t i = 0; i < content_length; ++i) content.push_back('2');
227+
butil::IOBuf content;
228+
for (size_t i = 0; i < content_length; ++i) {
229+
content.push_back('2');
230+
}
229231
butil::IOBuf request;
230232
request.append(header);
231233
request.append(content);
232234

233235
brpc::HttpMessage http_message;
234236
ASSERT_TRUE(http_message.ParseFromIOBuf(request) >= 0);
235237
ASSERT_TRUE(http_message.Completed());
238+
ASSERT_EQ(content, http_message.body());
236239
ASSERT_EQ(content, http_message.body().to_string());
237240
ASSERT_EQ("text/plain", http_message.header().content_type());
238241
}

0 commit comments

Comments
 (0)