From df59f067345e3a45be7637122ba8566009aab830 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 13 Jan 2010 08:41:01 -0800 Subject: [PATCH] recvMsg shouldn't return array for efficiency. --- lib/net.js | 34 ++++++++++++++----------------- src/node_net2.cc | 52 ++++++++++++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 41 deletions(-) diff --git a/lib/net.js b/lib/net.js index f513a9a802..2604cb5c6b 100644 --- a/lib/net.js +++ b/lib/net.js @@ -49,39 +49,35 @@ function Socket (peerInfo) { debug('recvBuffer.used ' + self.recvBuffer.used); var bytesRead; - var receivedFd = -1; if (self.type == "unix") { - var msgInfo = recvMsg(self.fd, - self.recvBuffer, - self.recvBuffer.used, - self.recvBuffer.length - self.recvBuffer.used); - bytesRead = msgInfo[0]; - receivedFd = msgInfo[1]; - debug('receivedFd ' + receivedFd); + bytesRead = recvMsg(self.fd, + self.recvBuffer, + self.recvBuffer.used, + self.recvBuffer.length - self.recvBuffer.used); + debug('recvMsg.fd ' + recvMsg.fd); + if (recvMsg.fd) { + self.emit('fd', recvMsg.fd); + } } else { bytesRead = read(self.fd, self.recvBuffer, self.recvBuffer.used, self.recvBuffer.length - self.recvBuffer.used); } + debug('bytesRead ' + bytesRead + '\n'); - if (bytesRead == 0) { + if (!recvMsg.fd && bytesRead == 0) { self.readable = false; self.readWatcher.stop(); self.emit('eof'); if (!self.writable) self.forceClose(); - } else { - if (receivedFd == -1) { - var slice = self.recvBuffer.slice(self.recvBuffer.used, - self.recvBuffer.used + bytesRead); - self.recvBuffer.used += bytesRead; - self.emit('data', slice); - } else { - self.recvBuffer.used += bytesRead; - self.emit('fd', receivedFd); - } + } else if (bytesRead > 0) { + var slice = self.recvBuffer.slice(self.recvBuffer.used, + self.recvBuffer.used + bytesRead); + self.recvBuffer.used += bytesRead; + self.emit('data', slice); } }; self.readable = false; diff --git a/src/node_net2.cc b/src/node_net2.cc index 4e52cddb4b..47e8da3cf0 100644 --- a/src/node_net2.cc +++ b/src/node_net2.cc @@ -45,6 +45,8 @@ static Persistent type_symbol; static Persistent tcp_symbol; static Persistent unix_symbol; +static Persistent recv_msg_template; + #define FD_ARG(a) \ if (!(a)->IsInt32()) { \ return ThrowException(Exception::TypeError( \ @@ -517,7 +519,10 @@ static Handle Read(const Arguments& args) { return scope.Close(Integer::New(bytes_read)); } -// bytesRead, receivedFd = t.recvMsg(fd, buffer, offset, length) +// bytesRead = t.recvMsg(fd, buffer, offset, length) +// if (recvMsg.fd) { +// receivedFd = recvMsg.fd; +// } static Handle RecvMsg(const Arguments& args) { HandleScope scope; @@ -528,7 +533,7 @@ static Handle RecvMsg(const Arguments& args) { FD_ARG(args[0]) - if (!IsBuffer(args[1])) { + if (!IsBuffer(args[1])) { return ThrowException(Exception::TypeError( String::New("Second argument should be a buffer"))); } @@ -547,23 +552,21 @@ static Handle RecvMsg(const Arguments& args) { String::New("Length is extends beyond buffer"))); } - struct iovec iov[1]; - struct msghdr msg; int received_fd; - char control_msg[CMSG_SPACE(sizeof(received_fd))]; - struct cmsghdr *cmsg; - - // TODO: zero out control_msg ? + struct iovec iov[1]; iov[0].iov_base = buffer_p(buffer, off); - iov[0].iov_len = buffer_remaining(buffer, off); + iov[0].iov_len = len; + + struct msghdr msg; msg.msg_iov = iov; msg.msg_iovlen = 1; msg.msg_name = NULL; msg.msg_namelen = 0; /* Set up to receive a descriptor even if one isn't in the message */ - msg.msg_control = (void *) control_msg; - msg.msg_controllen = CMSG_LEN(sizeof(received_fd)); + char cmsg_space[64]; // should be big enough + msg.msg_controllen = 64; + msg.msg_control = (void *) cmsg_space; ssize_t bytes_read = recvmsg(fd, &msg, 0); @@ -572,20 +575,22 @@ static Handle RecvMsg(const Arguments& args) { return ThrowException(ErrnoException(errno, "recvMsg")); } - // Return array of [bytesRead, fd] with fd == -1 if there was no FD - Local a = Array::New(2); - a->Set(Integer::New(0), Integer::New(bytes_read)); + // Why not return a two element array here [bytesRead, fd]? Because + // creating an object for each recvmsg() action is heavy. Instead we just + // assign the recved fd to a globalally accessable variable (recvMsg.fd) + // that the wrapper can pick up. Since we're single threaded, this is not + // a problem - just make sure to copy out that variable before the next + // call to recvmsg(). - cmsg = CMSG_FIRSTHDR(&msg); - if (cmsg->cmsg_type == SCM_RIGHTS) { + struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg); + if (cmsg && cmsg->cmsg_type == SCM_RIGHTS) { received_fd = *(int *) CMSG_DATA(cmsg); - } - else { - received_fd = -1; + recv_msg_template->GetFunction()->Set(fd_symbol, Integer::New(received_fd)); + } else { + recv_msg_template->GetFunction()->Set(fd_symbol, Null()); } - a->Set(Integer::New(1), Integer::New(received_fd)); - return scope.Close(a); + return scope.Close(Integer::New(bytes_read)); } @@ -904,7 +909,10 @@ void InitNet2(Handle target) { NODE_SET_METHOD(target, "read", Read); NODE_SET_METHOD(target, "sendFD", SendFD); - NODE_SET_METHOD(target, "recvMsg", RecvMsg); + + recv_msg_template = + Persistent::New(FunctionTemplate::New(RecvMsg)); + target->Set(String::NewSymbol("recvMsg"), recv_msg_template->GetFunction()); NODE_SET_METHOD(target, "socket", Socket); NODE_SET_METHOD(target, "close", Close);