Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 7e9204f

Browse files
mcollinaaduh95
authored andcommitted
http: use intrusive lists in ConnectionsList
Every HTTP message was performing multiple erase and insert operations on two std::set instances ordered by a mutating key, showing up as ~2% of CPU cycles in a hello-world server profile due to red-black tree rebalancing and node allocations. Replace both sets with intrusive doubly-linked lists. Membership in the list of all connections no longer changes per message, and updating the active connections list is now O(1) with no allocations. Appending to the tail keeps the active list ordered by last_message_start_ because uv_hrtime() is monotonic. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #65296 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4de7e63 commit 7e9204f

1 file changed

Lines changed: 82 additions & 71 deletions

File tree

β€Žsrc/node_http_parser.ccβ€Ž

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -246,55 +246,63 @@ struct StringPtr {
246246
size_t size_ = 0;
247247
};
248248

249-
structParserComparator {
250-
booloperator()(const Parser* lhs, const Parser* rhs) const;
249+
// Intrusive doubly-linked list node, linked to itself when not in a list.
250+
structParserListNode {
251+
ParserListNode* prev = this;
252+
ParserListNode* next = this;
253+
254+
ParserListNode() = default;
255+
~ParserListNode() { Remove(); }
256+
257+
ParserListNode(const ParserListNode&) = delete;
258+
ParserListNode& operator=(const ParserListNode&) = delete;
259+
260+
voidRemove() {
261+
prev->next = next;
262+
next->prev = prev;
263+
prev = this;
264+
next = this;
265+
}
251266
};
252267

253268
classConnectionsList : publicBaseObject {
254269
public:
255-
staticvoidNew(const FunctionCallbackInfo<Value>& args);
270+
staticvoidNew(const FunctionCallbackInfo<Value>& args);
256271

257-
staticvoidAll(const FunctionCallbackInfo<Value>& args);
272+
staticvoidAll(const FunctionCallbackInfo<Value>& args);
258273

259-
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
274+
staticvoidIdle(const FunctionCallbackInfo<Value>& args);
260275

261-
staticvoidActive(const FunctionCallbackInfo<Value>& args);
276+
staticvoidActive(const FunctionCallbackInfo<Value>& args);
262277

263-
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
278+
staticvoidExpired(const FunctionCallbackInfo<Value>& args);
264279

265-
voidPush(Parser* parser) {
266-
all_connections_.insert(parser);
267-
}
280+
inlinevoidPush(Parser* parser);
268281

269-
voidPop(Parser* parser) {
270-
all_connections_.erase(parser);
271-
}
282+
inlinevoidPop(Parser* parser);
272283

273-
voidPushActive(Parser* parser) {
274-
active_connections_.insert(parser);
275-
}
284+
inlinevoidPushActive(Parser* parser);
276285

277-
voidPopActive(Parser* parser) {
278-
active_connections_.erase(parser);
279-
}
286+
inlinevoidPopActive(Parser* parser);
280287

281-
SET_NO_MEMORY_INFO()
282-
SET_MEMORY_INFO_NAME(ConnectionsList)
283-
SET_SELF_SIZE(ConnectionsList)
288+
SET_NO_MEMORY_INFO()
289+
SET_MEMORY_INFO_NAME(ConnectionsList)
290+
SET_SELF_SIZE(ConnectionsList)
284291

285292
private:
286-
ConnectionsList(Environment* env, Local<Object> object)
293+
ConnectionsList(Environment* env, Local<Object> object)
287294
: BaseObject(env, object) {
288-
MakeWeak();
289-
}
295+
MakeWeak();
296+
}
290297

291-
std::set<Parser*, ParserComparator> all_connections_;
292-
std::set<Parser*, ParserComparator> active_connections_;
298+
// active_connections_ is ordered by last_message_start_, as parsers are
299+
// appended right after it is assigned from the monotonic uv_hrtime().
300+
ParserListNode all_connections_;
301+
ParserListNode active_connections_;
293302
};
294303

295304
classParser : publicAsyncWrap, publicStreamListener {
296305
friendclassConnectionsList;
297-
friendstructParserComparator;
298306

299307
public:
300308
Parser(BindingData* binding_data, Local<Object> wrap)
@@ -308,13 +316,6 @@ class Parser : public AsyncWrap, public StreamListener {
308316
SET_SELF_SIZE(Parser)
309317

310318
int on_message_begin() {
311-
// Important: Pop from the lists BEFORE resetting the last_message_start_
312-
// otherwise std::set.erase will fail.
313-
if (connectionsList_ != nullptr) {
314-
connectionsList_->Pop(this);
315-
connectionsList_->PopActive(this);
316-
}
317-
318319
num_fields_ = num_values_ = 0;
319320
headers_completed_ = false;
320321
chunk_extensions_nread_ = 0;
@@ -326,7 +327,6 @@ class Parser : public AsyncWrap, public StreamListener {
326327
max_header_pairs_ = -1;
327328

328329
if (connectionsList_ != nullptr) {
329-
connectionsList_->Push(this);
330330
connectionsList_->PushActive(this);
331331
}
332332

@@ -345,7 +345,6 @@ class Parser : public AsyncWrap, public StreamListener {
345345
return0;
346346
}
347347

348-
349348
inton_url(constchar* at, size_t length) {
350349
int rv = TrackHeader(length);
351350
if (rv != 0) {
@@ -544,19 +543,12 @@ class Parser : public AsyncWrap, public StreamListener {
544543
inton_message_complete() {
545544
HandleScope scope(env()->isolate());
546545

547-
// Important: Pop from the lists BEFORE resetting the last_message_start_
548-
// otherwise std::set.erase will fail.
549546
if (connectionsList_ != nullptr) {
550-
connectionsList_->Pop(this);
551547
connectionsList_->PopActive(this);
552548
}
553549

554550
last_message_start_ = 0;
555551

556-
if (connectionsList_ != nullptr) {
557-
connectionsList_->Push(this);
558-
}
559-
560552
if (num_fields_)
561553
Flush(); // Flush trailing HTTP headers.
562554

@@ -742,8 +734,6 @@ class Parser : public AsyncWrap, public StreamListener {
742734
// server.timeout is left to the default value of zero.
743735
parser->last_message_start_ = uv_hrtime();
744736

745-
// Important: Push into the lists AFTER setting the last_message_start_
746-
// otherwise std::set.erase will fail later.
747737
parser->connectionsList_->Push(parser);
748738
parser->connectionsList_->PushActive(parser);
749739
} else {
@@ -1122,6 +1112,8 @@ class Parser : public AsyncWrap, public StreamListener {
11221112
uint64_t max_http_header_size_;
11231113
uint64_t last_message_start_;
11241114
ConnectionsList* connectionsList_;
1115+
ParserListNode all_node_;
1116+
ParserListNode active_node_;
11251117

11261118
BaseObjectPtr<BindingData> binding_data_;
11271119

@@ -1149,18 +1141,34 @@ class Parser : public AsyncWrap, public StreamListener {
11491141
staticconstllhttp_settings_t settings;
11501142
};
11511143

1152-
boolParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
1153-
if (lhs->last_message_start_ == 0 && rhs->last_message_start_ == 0) {
1154-
// When both parsers are idle, guarantee strict order by
1155-
// comparing pointers as ints.
1156-
return lhs < rhs;
1157-
} elseif (lhs->last_message_start_ == 0) {
1158-
returntrue;
1159-
} elseif (rhs->last_message_start_ == 0) {
1160-
returnfalse;
1161-
}
1144+
namespace {
1145+
1146+
// Append `node` at the tail of the list headed by `head`, unlinking it from
1147+
// any list it is currently in.
1148+
voidListPushBack(ParserListNode* head, ParserListNode* node) {
1149+
node->Remove();
1150+
node->prev = head->prev;
1151+
node->next = head;
1152+
head->prev->next = node;
1153+
head->prev = node;
1154+
}
1155+
1156+
} // anonymous namespace
11621157

1163-
return lhs->last_message_start_ < rhs->last_message_start_;
1158+
voidConnectionsList::Push(Parser* parser) {
1159+
ListPushBack(&all_connections_, &parser->all_node_);
1160+
}
1161+
1162+
voidConnectionsList::Pop(Parser* parser) {
1163+
parser->all_node_.Remove();
1164+
}
1165+
1166+
voidConnectionsList::PushActive(Parser* parser) {
1167+
ListPushBack(&active_connections_, &parser->active_node_);
1168+
}
1169+
1170+
voidConnectionsList::PopActive(Parser* parser) {
1171+
parser->active_node_.Remove();
11641172
}
11651173

11661174
voidConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
@@ -1178,8 +1186,10 @@ void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
11781186
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11791187

11801188
LocalVector<Value> result(isolate);
1181-
result.reserve(list->all_connections_.size());
1182-
for (auto parser : list->all_connections_) {
1189+
for (ParserListNode* node = list->all_connections_.next;
1190+
node != &list->all_connections_;
1191+
node = node->next) {
1192+
Parser* parser = ContainerOf(&Parser::all_node_, node);
11831193
result.emplace_back(parser->object());
11841194
}
11851195

@@ -1195,8 +1205,10 @@ void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
11951205
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
11961206

11971207
LocalVector<Value> result(isolate);
1198-
result.reserve(list->all_connections_.size());
1199-
for (auto parser : list->all_connections_) {
1208+
for (ParserListNode* node = list->all_connections_.next;
1209+
node != &list->all_connections_;
1210+
node = node->next) {
1211+
Parser* parser = ContainerOf(&Parser::all_node_, node);
12001212
if (parser->last_message_start_ == 0 || !parser->received_data_) {
12011213
result.emplace_back(parser->object());
12021214
}
@@ -1214,8 +1226,10 @@ void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
12141226
ASSIGN_OR_RETURN_UNWRAP(&list, args.This());
12151227

12161228
LocalVector<Value> result(isolate);
1217-
result.reserve(list->active_connections_.size());
1218-
for (auto parser : list->active_connections_) {
1229+
for (ParserListNode* node = list->active_connections_.next;
1230+
node != &list->active_connections_;
1231+
node = node->next) {
1232+
Parser* parser = ContainerOf(&Parser::active_node_, node);
12191233
result.emplace_back(parser->object());
12201234
}
12211235

@@ -1259,14 +1273,11 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12591273
return args.GetReturnValue().Set(Array::New(isolate, 0));
12601274
}
12611275

1262-
auto iter = list->active_connections_.begin();
1263-
auto end = list->active_connections_.end();
1264-
12651276
LocalVector<Value> result(isolate);
1266-
result.reserve(list->active_connections_.size());
1267-
while (iter != end) {
1268-
Parser* parser = *iter;
1269-
iter++;
1277+
ParserListNode* node = list->active_connections_.next;
1278+
while (node != &list->active_connections_) {
1279+
Parser* parser = ContainerOf(&Parser::active_node_, node);
1280+
node = node->next;
12701281

12711282
// Check for expiration.
12721283
if (
@@ -1278,7 +1289,7 @@ void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
12781289
) {
12791290
result.emplace_back(parser->object());
12801291

1281-
list->active_connections_.erase(parser);
1292+
parser->active_node_.Remove();
12821293
}
12831294
}
12841295

0 commit comments

Comments
Β (0)