Skip to content

Commit

Permalink
src: avoid manual memory management in inspector
Browse files Browse the repository at this point in the history
Make the inspector code easier to reason about by restructuring it
to avoid manual memory allocation and copying as much as possible.

An amusing side effect is that it reduces the total amount of memory
used in the test suite.

Before:

    $ valgrind ./out/Release/cctest 2>&1 | grep 'total heap' | cut -c31-
    1,017 allocs, 1,017 frees, 21,695,456 allocated

After:

    $ valgrind ./out/Release/cctest 2>&1 | grep 'total heap' | cut -c31-
    869 allocs, 869 frees, 14,484,641 bytes allocated

PR-URL: nodejs#7906
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis committed Aug 1, 2016
1 parent 0190db4 commit c8c1f96
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 228 deletions.
31 changes: 14 additions & 17 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ void PrintDebuggerReadyMessage(int port) {
port, DEVTOOLS_HASH, port);
}

bool AcceptsConnection(inspector_socket_t* socket, const char* path) {
return strncmp(DEVTOOLS_PATH, path, sizeof(DEVTOOLS_PATH)) == 0;
bool AcceptsConnection(inspector_socket_t* socket, const std::string& path) {
return 0 == path.compare(0, sizeof(DEVTOOLS_PATH) - 1, DEVTOOLS_PATH);
}

void DisposeInspector(inspector_socket_t* socket, int status) {
Expand All @@ -63,10 +63,7 @@ void DisconnectAndDisposeIO(inspector_socket_t* socket) {
}

void OnBufferAlloc(uv_handle_t* handle, size_t len, uv_buf_t* buf) {
if (len > 0) {
buf->base = static_cast<char*>(malloc(len));
CHECK_NE(buf->base, nullptr);
}
buf->base = new char[len];
buf->len = len;
}

Expand Down Expand Up @@ -133,18 +130,19 @@ void SendTargentsListResponse(inspector_socket_t* socket, int port) {
SendHttpResponse(socket, buffer, len);
}

bool RespondToGet(inspector_socket_t* socket, const char* path, int port) {
bool RespondToGet(inspector_socket_t* socket, const std::string& path,
int port) {
const char PATH[] = "/json";
const char PATH_LIST[] = "/json/list";
const char PATH_VERSION[] = "/json/version";
const char PATH_ACTIVATE[] = "/json/activate/";
if (!strncmp(PATH_VERSION, path, sizeof(PATH_VERSION))) {
if (0 == path.compare(0, sizeof(PATH_VERSION) - 1, PATH_VERSION)) {
SendVersionResponse(socket);
} else if (!strncmp(PATH_LIST, path, sizeof(PATH_LIST)) ||
!strncmp(PATH, path, sizeof(PATH))) {
} else if (0 == path.compare(0, sizeof(PATH_LIST) - 1, PATH_LIST) ||
0 == path.compare(0, sizeof(PATH) - 1, PATH)) {
SendTargentsListResponse(socket, port);
} else if (!strncmp(path, PATH_ACTIVATE, sizeof(PATH_ACTIVATE) - 1) &&
atoi(path + (sizeof(PATH_ACTIVATE) - 1)) == getpid()) {
} else if (0 == path.compare(0, sizeof(PATH_ACTIVATE) - 1, PATH_ACTIVATE) &&
atoi(path.substr(sizeof(PATH_ACTIVATE) - 1).c_str()) == getpid()) {
const char TARGET_ACTIVATED[] = "Target activated";
SendHttpResponse(socket, TARGET_ACTIVATED, sizeof(TARGET_ACTIVATED) - 1);
} else {
Expand Down Expand Up @@ -181,7 +179,7 @@ class AgentImpl {
static void OnSocketConnectionIO(uv_stream_t* server, int status);
static bool OnInspectorHandshakeIO(inspector_socket_t* socket,
enum inspector_handshake_event state,
const char* path);
const std::string& path);
static void WriteCbIO(uv_async_t* async);

void WorkerRunIO();
Expand Down Expand Up @@ -388,7 +386,6 @@ void AgentImpl::ThreadCbIO(void* agent) {
void AgentImpl::OnSocketConnectionIO(uv_stream_t* server, int status) {
if (status == 0) {
inspector_socket_t* socket = new inspector_socket_t();
memset(socket, 0, sizeof(*socket));
socket->data = server->data;
if (inspector_accept(server, socket,
AgentImpl::OnInspectorHandshakeIO) != 0) {
Expand All @@ -399,8 +396,8 @@ void AgentImpl::OnSocketConnectionIO(uv_stream_t* server, int status) {

// static
bool AgentImpl::OnInspectorHandshakeIO(inspector_socket_t* socket,
enum inspector_handshake_event state,
const char* path) {
enum inspector_handshake_event state,
const std::string& path) {
AgentImpl* agent = static_cast<AgentImpl*>(socket->data);
switch (state) {
case kInspectorHandshakeHttpGet:
Expand Down Expand Up @@ -443,7 +440,7 @@ void AgentImpl::OnRemoteDataIO(inspector_socket_t* socket,
DisconnectAndDisposeIO(socket);
}
if (buf) {
free(buf->base);
delete[] buf->base;
}
pause_cond_.Broadcast(scoped_lock);
}
Expand Down
Loading

0 comments on commit c8c1f96

Please sign in to comment.