Commit e15801b5 authored by teravest@chromium.org's avatar teravest@chromium.org

Pepper: Narrow locking at ld subprocess start.

Currently, subprocess_mu_ is held the entire time the ld subprocess is
starting. This blocks a larger refactor that I'd like to make as part of nexe
loading.

I'd like to change ServiceRuntime::LoadModule() to be asynchronous to simplify
threading behavior on the plugin side when loading nexe modules. However, this
requires that many methods are made asynchronous, including
Plugin::LoadHelperNaClModule().

PnaclTranslateThread will need to use a pattern similar to WaitForSelLdrStart()
to resume execution on a background thread. However, I don't like the idea of
introducing another mutex and condvar to deal with while holding
subprocess_mu_.

This change narrows the time that subprocess_mu_ is held to make that refactor
possible. This follows a similar change made for the llc subprocess.

BUG=333950
R=bbudge@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274557 0039d316-1c4b-4281-b951-d872f2087c98
parent a1268362
...@@ -339,29 +339,38 @@ bool PnaclTranslateThread::RunLdSubprocess() { ...@@ -339,29 +339,38 @@ bool PnaclTranslateThread::RunLdSubprocess() {
nacl::DescWrapper* ld_out_file = nexe_file_->write_wrapper(); nacl::DescWrapper* ld_out_file = nexe_file_->write_wrapper();
pp::Core* core = pp::Module::Get()->core(); pp::Core* core = pp::Module::Get()->core();
int64_t ld_start_time = NaClGetTimeOfDayMicroseconds();
PP_FileHandle ld_file_handle = resources_->TakeLdFileHandle();
// On success, ownership of ld_file_handle is transferred.
nacl::scoped_ptr<NaClSubprocess> ld_subprocess(
plugin_->LoadHelperNaClModule(resources_->GetLlcUrl(),
ld_file_handle,
&error_info));
if (ld_subprocess.get() == NULL) {
if (ld_file_handle != PP_kInvalidFileHandle)
CloseFileHandle(ld_file_handle);
TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP,
"Link process could not be created: " +
error_info.message());
return false;
}
core->CallOnMainThread(0,
coordinator_->GetUMATimeCallback(
"NaCl.Perf.PNaClLoadTime.LoadLinker",
NaClGetTimeOfDayMicroseconds() - ld_start_time),
PP_OK);
{ {
// Create LD process
nacl::MutexLocker ml(&subprocess_mu_); nacl::MutexLocker ml(&subprocess_mu_);
int64_t ld_start_time = NaClGetTimeOfDayMicroseconds(); // If we received a call to AbortSubprocesses() before we had a chance to
PP_FileHandle ld_file_handle = resources_->TakeLdFileHandle(); // set llc_subprocess_, shut down and clean up the subprocess started here.
if (subprocesses_aborted_) {
// On success, ownership of ld_file_handle is transferred. ld_subprocess->service_runtime()->Shutdown();
ld_subprocess_.reset(plugin_->LoadHelperNaClModule(
resources_->GetLdUrl(), ld_file_handle, &error_info));
if (ld_subprocess_ == NULL) {
if (ld_file_handle != PP_kInvalidFileHandle)
CloseFileHandle(ld_file_handle);
TranslateFailed(PP_NACL_ERROR_PNACL_LD_SETUP,
"Link process could not be created: " +
error_info.message());
return false; return false;
} }
DCHECK(ld_subprocess_.get() == NULL);
ld_subprocess_.swap(ld_subprocess);
ld_subprocess_active_ = true; ld_subprocess_active_ = true;
core->CallOnMainThread(0,
coordinator_->GetUMATimeCallback(
"NaCl.Perf.PNaClLoadTime.LoadLinker",
NaClGetTimeOfDayMicroseconds() - ld_start_time),
PP_OK);
} }
int64_t link_start_time = NaClGetTimeOfDayMicroseconds(); int64_t link_start_time = NaClGetTimeOfDayMicroseconds();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment