Skip to content

Commit

Permalink
Increase MessageAttachmentSet::kMaxDescriptorsPerMessage to 128
Browse files Browse the repository at this point in the history 
so that dynamically linked NaCl applications like ARC (App Runtime
for Chrome) start faster.

ARC is a dynamically linked NaCl application which lists more than
70 DSO files in its nmf file. crrev.com/319931 allowed Chrome to
pre-open up to 2 DSO files when it spawns a NaCl plugin process so
that the plugin process can open/read/map the DSO files without
issuing any Chrome IPC. This CL increase the number of DSO files
Chrome can pre-open from 2 to 120+.

BUG=348232
BUG=nativeclient:3802
TEST=ipc_tests, browser_tests (*PackagedAppTest.SuccessfulLoad)
TEST=manually tested non-SFI ARC apps

Review URL: https://codereview.chromium.org/998833002

Cr-Commit-Position: refs/heads/master@{#320831}
  • Loading branch information
@yusukes
yusukes authored and Commit bot committed Mar 17, 2015
1 parent c970478 commit eaa3c5b
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 12 deletions.
13 changes: 10 additions & 3 deletions components/nacl/browser/nacl_host_message_filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "content/public/browser/plugin_service.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "ipc/ipc_message_attachment_set.h"
#include "ipc/ipc_platform_file.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
Expand All @@ -25,9 +26,15 @@ namespace nacl {
namespace {

// The maximum number of resource file handles NaClProcessMsg_Start message
// can have.
// TODO(yusukes): Increase the number.
const size_t kMaxPreOpenResourceFiles = 2;
// can have. Currently IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage
// is 128 and NaCl sends 5 handles for other purposes, hence 123.
const size_t kMaxPreOpenResourceFiles = 123;

#if defined(OS_POSIX)
static_assert(kMaxPreOpenResourceFiles ==
IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage - 5,
"kMaxPreOpenResourceFiles is not up to date");
#endif

ppapi::PpapiPermissions GetNaClPermissions(
uint32 permission_bits,
Expand Down
2 changes: 1 addition & 1 deletion ipc/ipc_channel_nacl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ bool ReadDataOnReaderThread(int pipe, MessageContents* contents) {
return false;

contents->data.resize(Channel::kReadBufferSize);
contents->fds.resize(MessageAttachmentSet::kMaxDescriptorsPerMessage);
contents->fds.resize(NACL_ABI_IMC_DESC_MAX);

NaClAbiNaClImcMsgIoVec iov = { &contents->data[0], contents->data.size() };
NaClAbiNaClImcMsgHdr msg = {
Expand Down
2 changes: 1 addition & 1 deletion ipc/ipc_message_attachment_set.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class IPC_EXPORT MessageAttachmentSet
//
// In debugging mode, it's a fatal error to try and add more than this number
// of descriptors to a MessageAttachmentSet.
static const size_t kMaxDescriptorsPerMessage = 7;
static const size_t kMaxDescriptorsPerMessage = 128;

// ---------------------------------------------------------------------------
// Interfaces for transmission...
Expand Down
4 changes: 2 additions & 2 deletions ipc/ipc_send_fds_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ extern "C" {

namespace {

const unsigned kNumFDsToSend = 7; // per message
const unsigned kNumMessages = 20;
const unsigned kNumFDsToSend = 128; // per message
const unsigned kNumMessages = 3;
const char* kDevZeroPath = "/dev/zero";

#if defined(OS_POSIX)
Expand Down
126 changes: 124 additions & 2 deletions ppapi/tests/extensions/extensions.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@
'packaged_app/test_file2.txt',
],
'create_nmf_args_portable': [
# Add lots of "files" entries to make sure that open_resource can
# handle more files than FileDescriptorSet::kMaxDescriptorsPerMessage.
# Add 129 "files" entries to make sure that open_resource can handle
# more files than IPC::MessageAttachmentSet::kMaxDescriptorsPerMessage
# which is currently 128.
'-xtest_file0:test_file.txt',
'-xtest_file1:test_file2.txt',
'-xtest_file2:test_file.txt',
Expand All @@ -79,6 +80,127 @@
'-xtest_file5:test_file2.txt',
'-xtest_file6:test_file.txt',
'-xtest_file7:test_file2.txt',
'-xtest_file8:test_file.txt',
'-xtest_file9:test_file2.txt',
'-xtest_file10:test_file.txt',
'-xtest_file11:test_file2.txt',
'-xtest_file12:test_file.txt',
'-xtest_file13:test_file2.txt',
'-xtest_file14:test_file.txt',
'-xtest_file15:test_file2.txt',
'-xtest_file16:test_file.txt',
'-xtest_file17:test_file2.txt',
'-xtest_file18:test_file.txt',
'-xtest_file19:test_file2.txt',
'-xtest_file20:test_file.txt',
'-xtest_file21:test_file2.txt',
'-xtest_file22:test_file.txt',
'-xtest_file23:test_file2.txt',
'-xtest_file24:test_file.txt',
'-xtest_file25:test_file2.txt',
'-xtest_file26:test_file.txt',
'-xtest_file27:test_file2.txt',
'-xtest_file28:test_file.txt',
'-xtest_file29:test_file2.txt',
'-xtest_file30:test_file.txt',
'-xtest_file31:test_file2.txt',
'-xtest_file32:test_file.txt',
'-xtest_file33:test_file2.txt',
'-xtest_file34:test_file.txt',
'-xtest_file35:test_file2.txt',
'-xtest_file36:test_file.txt',
'-xtest_file37:test_file2.txt',
'-xtest_file38:test_file.txt',
'-xtest_file39:test_file2.txt',
'-xtest_file40:test_file.txt',
'-xtest_file41:test_file2.txt',
'-xtest_file42:test_file.txt',
'-xtest_file43:test_file2.txt',
'-xtest_file44:test_file.txt',
'-xtest_file45:test_file2.txt',
'-xtest_file46:test_file.txt',
'-xtest_file47:test_file2.txt',
'-xtest_file48:test_file.txt',
'-xtest_file49:test_file2.txt',
'-xtest_file50:test_file.txt',
'-xtest_file51:test_file2.txt',
'-xtest_file52:test_file.txt',
'-xtest_file53:test_file2.txt',
'-xtest_file54:test_file.txt',
'-xtest_file55:test_file2.txt',
'-xtest_file56:test_file.txt',
'-xtest_file57:test_file2.txt',
'-xtest_file58:test_file.txt',
'-xtest_file59:test_file2.txt',
'-xtest_file60:test_file.txt',
'-xtest_file61:test_file2.txt',
'-xtest_file62:test_file.txt',
'-xtest_file63:test_file2.txt',
'-xtest_file64:test_file.txt',
'-xtest_file65:test_file2.txt',
'-xtest_file66:test_file.txt',
'-xtest_file67:test_file2.txt',
'-xtest_file68:test_file.txt',
'-xtest_file69:test_file2.txt',
'-xtest_file70:test_file.txt',
'-xtest_file71:test_file2.txt',
'-xtest_file72:test_file.txt',
'-xtest_file73:test_file2.txt',
'-xtest_file74:test_file.txt',
'-xtest_file75:test_file2.txt',
'-xtest_file76:test_file.txt',
'-xtest_file77:test_file2.txt',
'-xtest_file78:test_file.txt',
'-xtest_file79:test_file2.txt',
'-xtest_file80:test_file.txt',
'-xtest_file81:test_file2.txt',
'-xtest_file82:test_file.txt',
'-xtest_file83:test_file2.txt',
'-xtest_file84:test_file.txt',
'-xtest_file85:test_file2.txt',
'-xtest_file86:test_file.txt',
'-xtest_file87:test_file2.txt',
'-xtest_file88:test_file.txt',
'-xtest_file89:test_file2.txt',
'-xtest_file90:test_file.txt',
'-xtest_file91:test_file2.txt',
'-xtest_file92:test_file.txt',
'-xtest_file93:test_file2.txt',
'-xtest_file94:test_file.txt',
'-xtest_file95:test_file2.txt',
'-xtest_file96:test_file.txt',
'-xtest_file97:test_file2.txt',
'-xtest_file98:test_file.txt',
'-xtest_file99:test_file2.txt',
'-xtest_file100:test_file.txt',
'-xtest_file101:test_file2.txt',
'-xtest_file102:test_file.txt',
'-xtest_file103:test_file2.txt',
'-xtest_file104:test_file.txt',
'-xtest_file105:test_file2.txt',
'-xtest_file106:test_file.txt',
'-xtest_file107:test_file2.txt',
'-xtest_file108:test_file.txt',
'-xtest_file109:test_file2.txt',
'-xtest_file110:test_file.txt',
'-xtest_file111:test_file2.txt',
'-xtest_file112:test_file.txt',
'-xtest_file113:test_file2.txt',
'-xtest_file114:test_file.txt',
'-xtest_file115:test_file2.txt',
'-xtest_file116:test_file.txt',
'-xtest_file117:test_file2.txt',
'-xtest_file118:test_file.txt',
'-xtest_file119:test_file2.txt',
'-xtest_file120:test_file.txt',
'-xtest_file121:test_file2.txt',
'-xtest_file122:test_file.txt',
'-xtest_file123:test_file2.txt',
'-xtest_file124:test_file.txt',
'-xtest_file125:test_file2.txt',
'-xtest_file126:test_file.txt',
'-xtest_file127:test_file2.txt',
'-xtest_file128:test_file.txt',
],
},
'conditions': [
Expand Down
6 changes: 3 additions & 3 deletions ppapi/tests/extensions/packaged_app/test_packaged_app.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ namespace {
std::string g_last_error;
pp::Instance* g_instance = NULL;

// This should be the same as FileDescriptorSet::kMaxDescriptorsPerMessage in
// ipc/file_descriptor_set_posix.h.
// This should be the same as MessageAttachmentSet::kMaxDescriptorsPerMessage in
// ipc/ipc_message_attachment_set.h.
// TODO(yusukes): Once all the NaCl toolchains support C++11, Add
// #include "ipc/file_descriptor_set_posix.h" and remove the constant.
const size_t kMaxDescriptorsPerMessage = 7;
const size_t kMaxDescriptorsPerMessage = 128;

// Returns true if the resource file whose name is |key| exists and its content
// matches |content|.
Expand Down

0 comments on commit eaa3c5b

Please sign in to comment.