Skip to content

Commit

Permalink
src: move http parser state out of Environment
Browse files Browse the repository at this point in the history
Moves state that is specific to HTTP/1 into the HTTP/1 implementation
as a cleanup.

PR-URL: #32538
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax committed Apr 6, 2020
1 parent c47d042 commit 1216e8f
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 38 deletions.
17 changes: 0 additions & 17 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -580,23 +580,6 @@ inline double Environment::get_default_trigger_async_id() {
return default_trigger_async_id;
}

inline char* Environment::http_parser_buffer() const {
return http_parser_buffer_;
}

inline void Environment::set_http_parser_buffer(char* buffer) {
CHECK_NULL(http_parser_buffer_); // Should be set only once.
http_parser_buffer_ = buffer;
}

inline bool Environment::http_parser_buffer_in_use() const {
return http_parser_buffer_in_use_;
}

inline void Environment::set_http_parser_buffer_in_use(bool in_use) {
http_parser_buffer_in_use_ = in_use;
}

inline AliasedFloat64Array* Environment::fs_stats_field_array() {
return &fs_stats_field_array_;
}
Expand Down
2 changes: 0 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,6 @@ Environment::~Environment() {
tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get());
}

delete[] http_parser_buffer_;

TRACE_EVENT_NESTABLE_ASYNC_END0(
TRACING_CATEGORY_NODE1(environment), "Environment", this);

Expand Down
7 changes: 0 additions & 7 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -1007,11 +1007,6 @@ class Environment : public MemoryRetainer {
inline uint32_t get_next_script_id();
inline uint32_t get_next_function_id();

inline char* http_parser_buffer() const;
inline void set_http_parser_buffer(char* buffer);
inline bool http_parser_buffer_in_use() const;
inline void set_http_parser_buffer_in_use(bool in_use);

EnabledDebugList* enabled_debug_list() { return &enabled_debug_list_; }

inline AliasedFloat64Array* fs_stats_field_array();
Expand Down Expand Up @@ -1368,8 +1363,6 @@ class Environment : public MemoryRetainer {
int handle_cleanup_waiting_ = 0;
int request_waiting_ = 0;

char* http_parser_buffer_ = nullptr;
bool http_parser_buffer_in_use_ = false;
EnabledDebugList enabled_debug_list_;

AliasedFloat64Array fs_stats_field_array_;
Expand Down
44 changes: 32 additions & 12 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ inline bool IsOWS(char c) {
return c == ' ' || c == '\t';
}

class BindingData : public BaseObject {
public:
BindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}

std::vector<char> parser_buffer;
bool parser_buffer_in_use = false;

void MemoryInfo(MemoryTracker* tracker) const override {
tracker->TrackField("parser_buffer", parser_buffer);
}
SET_SELF_SIZE(BindingData)
SET_MEMORY_INFO_NAME(BindingData)
};

// helper class for the Parser
struct StringPtr {
StringPtr() {
Expand Down Expand Up @@ -164,10 +178,11 @@ struct StringPtr {

class Parser : public AsyncWrap, public StreamListener {
public:
Parser(Environment* env, Local<Object> wrap)
: AsyncWrap(env, wrap),
Parser(BindingData* binding_data, Local<Object> wrap)
: AsyncWrap(binding_data->env(), wrap),
current_buffer_len_(0),
current_buffer_data_(nullptr) {
current_buffer_data_(nullptr),
binding_data_(binding_data) {
}


Expand Down Expand Up @@ -429,8 +444,8 @@ class Parser : public AsyncWrap, public StreamListener {
}

static void New(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
new Parser(env, args.This());
BindingData* binding_data = Unwrap<BindingData>(args.Data());
new Parser(binding_data, args.This());
}


Expand Down Expand Up @@ -605,14 +620,14 @@ class Parser : public AsyncWrap, public StreamListener {
// For most types of streams, OnStreamRead will be immediately after
// OnStreamAlloc, and will consume all data, so using a static buffer for
// reading is more efficient. For other streams, just use Malloc() directly.
if (env()->http_parser_buffer_in_use())
if (binding_data_->parser_buffer_in_use)
return uv_buf_init(Malloc(suggested_size), suggested_size);
env()->set_http_parser_buffer_in_use(true);
binding_data_->parser_buffer_in_use = true;

if (env()->http_parser_buffer() == nullptr)
env()->set_http_parser_buffer(new char[kAllocBufferSize]);
if (binding_data_->parser_buffer.empty())
binding_data_->parser_buffer.resize(kAllocBufferSize);

return uv_buf_init(env()->http_parser_buffer(), kAllocBufferSize);
return uv_buf_init(binding_data_->parser_buffer.data(), kAllocBufferSize);
}


Expand All @@ -622,8 +637,8 @@ class Parser : public AsyncWrap, public StreamListener {
// is free for re-use, or free() the data if it didn’t come from there
// in the first place.
auto on_scope_leave = OnScopeLeave([&]() {
if (buf.base == env()->http_parser_buffer())
env()->set_http_parser_buffer_in_use(false);
if (buf.base == binding_data_->parser_buffer.data())
binding_data_->parser_buffer_in_use = false;
else
free(buf.base);
});
Expand Down Expand Up @@ -863,6 +878,8 @@ class Parser : public AsyncWrap, public StreamListener {
uint64_t headers_timeout_;
uint64_t header_parsing_start_time_ = 0;

BaseObjectPtr<BindingData> binding_data_;

// These are helper functions for filling `http_parser_settings`, which turn
// a member function of Parser into a C-style HTTP parser callback.
template <typename Parser, Parser> struct Proxy;
Expand Down Expand Up @@ -903,6 +920,9 @@ void InitializeHttpParser(Local<Object> target,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Environment::BindingScope<BindingData> binding_scope(env);
if (!binding_scope) return;

Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount);
t->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "HTTPParser"));
Expand Down

0 comments on commit 1216e8f

Please sign in to comment.