Skip to content

Commit

Permalink
Bug 1643862 - Move channel buffers into separate heap allocations, r=jld
Browse files Browse the repository at this point in the history
Moving these out-of-line reduces some IPC::Channel::ChannelImpl slop.

Differential Revision: https://phabricator.services.mozilla.com/D79183
  • Loading branch information
Kashav Madan committed Jul 3, 2020
1 parent cfc4c0e commit 4fb30e0
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 31 deletions.
19 changes: 12 additions & 7 deletions ipc/chromium/src/chrome/common/ipc_channel_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,17 @@ Channel::ChannelImpl::ChannelImpl(int fd, Mode mode, Listener* listener)
}

void Channel::ChannelImpl::Init(Mode mode, Listener* listener) {
DCHECK(kControlBufferSlopBytes >= CMSG_SPACE(0));
// Verify that we fit in a "quantum-spaced" jemalloc bucket.
static_assert(sizeof(*this) <= 512, "Exceeded expected size class");

DCHECK(kControlBufferHeaderSize >= CMSG_SPACE(0));

mode_ = mode;
is_blocked_on_write_ = false;
partial_write_iter_.reset();
input_buf_offset_ = 0;
input_buf_ = mozilla::MakeUnique<char[]>(Channel::kReadBufferSize);
input_cmsg_buf_ = mozilla::MakeUnique<char[]>(kControlBufferSize);
server_listen_pipe_ = -1;
pipe_ = -1;
client_pipe_ = -1;
Expand Down Expand Up @@ -339,16 +344,16 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {

msg.msg_iov = &iov;
msg.msg_iovlen = 1;
msg.msg_control = input_cmsg_buf_;
msg.msg_control = input_cmsg_buf_.get();

for (;;) {
msg.msg_controllen = sizeof(input_cmsg_buf_);
msg.msg_controllen = kControlBufferSize;

if (pipe_ == -1) return false;

// In some cases the beginning of a message will be stored in input_buf_. We
// don't want to overwrite that, so we store the new data after it.
iov.iov_base = input_buf_ + input_buf_offset_;
iov.iov_base = input_buf_.get() + input_buf_offset_;
iov.iov_len = Channel::kReadBufferSize - input_buf_offset_;

// Read from pipe.
Expand Down Expand Up @@ -416,8 +421,8 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
}

// Process messages from input buffer.
const char* p = input_buf_;
const char* end = input_buf_ + input_buf_offset_ + bytes_read;
const char* p = input_buf_.get();
const char* end = input_buf_.get() + input_buf_offset_ + bytes_read;

// A pointer to an array of |num_fds| file descriptors which includes any
// fds that have spilled over from a previous read.
Expand Down Expand Up @@ -475,7 +480,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
// Move everything we have to the start of the buffer. We'll finish
// reading this message when we get more data. For now we leave it in
// input_buf_.
memmove(input_buf_, p, end - p);
memmove(input_buf_.get(), p, end - p);
input_buf_offset_ = end - p;

break;
Expand Down
37 changes: 18 additions & 19 deletions ipc/chromium/src/chrome/common/ipc_channel_posix.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,27 +101,26 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
// Messages to be sent are queued here.
std::queue<mozilla::UniquePtr<Message>> output_queue_;

// We read from the pipe into this buffer
char input_buf_[Channel::kReadBufferSize];
// We read from the pipe into these buffers.
size_t input_buf_offset_;

// We want input_cmsg_buf_ to be big enough to hold
// MAX_DESCRIPTORS_PER_MESSAGE worth of file descriptors. However, CMSG_SPACE
// is apparently not a constant on Macs, so we can't use it in the array size.
// Consequently, we pick a number here that is at least CMSG_SPACE(0) on all
// platforms. And we assert at runtime, in Channel::ChannelImpl::Init, that
mozilla::UniquePtr<char[]> input_buf_;
mozilla::UniquePtr<char[]> input_cmsg_buf_;

// The control message buffer will hold all of the file descriptors that will
// be read in during a single recvmsg call. Message::WriteFileDescriptor
// always writes one word of data for every file descriptor added to the
// message, and the number of file descriptors per message will not exceed
// MAX_DESCRIPTORS_PER_MESSAGE.
//
// This buffer also holds a control message header of size CMSG_SPACE(0)
// bytes. However, CMSG_SPACE is not a constant on Macs, so we can't use it
// here. Consequently, we pick a number here that is at least CMSG_SPACE(0) on
// all platforms. We assert at runtime, in Channel::ChannelImpl::Init, that
// it's big enough.
enum { kControlBufferSlopBytes = 32 };

// This is a control message buffer large enough to hold all the file
// descriptors that will be read in when reading Channel::kReadBufferSize
// bytes of data. Message::WriteFileDescriptor always writes one
// word of data for every file descriptor added to the message. The number of
// file descriptors per message will not exceed MAX_DESCRIPTORS_PER_MESSAGE.
// We add kControlBufferSlopBytes bytes for the control header.
char input_cmsg_buf_[FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE *
sizeof(int) +
kControlBufferSlopBytes];
static constexpr size_t kControlBufferHeaderSize = 32;
static constexpr size_t kControlBufferSize =
FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE * sizeof(int) +
kControlBufferHeaderSize;

// Large incoming messages that span multiple pipe buffers get built-up in the
// buffers of this message.
Expand Down
12 changes: 8 additions & 4 deletions ipc/chromium/src/chrome/common/ipc_channel_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id,
}

void Channel::ChannelImpl::Init(Mode mode, Listener* listener) {
// Verify that we fit in a "quantum-spaced" jemalloc bucket.
static_assert(sizeof(*this) <= 512, "Exceeded expected size class");

pipe_ = INVALID_HANDLE_VALUE;
listener_ = listener;
waiting_connect_ = (mode == MODE_SERVER);
processing_incoming_ = false;
closed_ = false;
output_queue_length_ = 0;
input_buf_offset_ = 0;
input_buf_ = mozilla::MakeUnique<char[]>(Channel::kReadBufferSize);
}

void Channel::ChannelImpl::OutputQueuePush(mozilla::UniquePtr<Message> msg) {
Expand Down Expand Up @@ -340,7 +344,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages(
if (INVALID_HANDLE_VALUE == pipe_) return false;

// Read from pipe...
BOOL ok = ReadFile(pipe_, input_buf_ + input_buf_offset_,
BOOL ok = ReadFile(pipe_, input_buf_.get() + input_buf_offset_,
Channel::kReadBufferSize - input_buf_offset_,
&bytes_read, &input_state_.context.overlapped);
if (!ok) {
Expand All @@ -361,8 +365,8 @@ bool Channel::ChannelImpl::ProcessIncomingMessages(

// Process messages from input buffer.

const char* p = input_buf_;
const char* end = input_buf_ + input_buf_offset_ + bytes_read;
const char* p = input_buf_.get();
const char* end = input_buf_.get() + input_buf_offset_ + bytes_read;

while (p < end) {
// Try to figure out how big the message is. Size is 0 if we haven't read
Expand All @@ -381,7 +385,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages(
// Move everything we have to the start of the buffer. We'll finish
// reading this message when we get more data. For now we leave it in
// input_buf_.
memmove(input_buf_, p, end - p);
memmove(input_buf_.get(), p, end - p);
input_buf_offset_ = end - p;

break;
Expand Down
2 changes: 1 addition & 1 deletion ipc/chromium/src/chrome/common/ipc_channel_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
mozilla::Maybe<Pickle::BufferList::IterImpl> partial_write_iter_;

// We read from the pipe into this buffer
char input_buf_[Channel::kReadBufferSize];
mozilla::UniquePtr<char[]> input_buf_;
size_t input_buf_offset_;

// Large incoming messages that span multiple pipe buffers get built-up in the
Expand Down

0 comments on commit 4fb30e0

Please sign in to comment.