Use a single, generic, global zygote to temporarily resolve perf regressions.

Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191, 580966, 580974
Committed: https://crrev.com/94807ee60fa56c118cac88b0f8f4e4e5231ef43f
Cr-Commit-Position: refs/heads/master@{#372709}

[Greg K, 2016-01-28 22:16:32]

Description was changed from

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974
==========

to

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974
==========

[Greg K, 2016-01-28 22:16:32]

kerrnel@chromium.org changed reviewers:
+ mdempsky@chromium.org

[Greg K, 2016-01-28 22:16:44]

Matthew, please review.

[mdempsky, 2016-01-28 23:25:40]

I think duplicating the same global variable value across multiple places is a
bad smell.  Also, it's desirable to avoid dynamic initialization of static
variables.

https://codereview.chromium.org/1643533006/diff/1/content/browser/browser_mai...
File content/browser/browser_main_loop.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/browser_mai...
content/browser/browser_main_loop.cc:230:
RenderProcessHostImpl::EarlyZygoteLaunch();
Before RenderProcessHostImpl::EarlyZygoteLaunch, add

  *GetGenericZygote() = CreateZygote();

https://codereview.chromium.org/1643533006/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.cc:553:
g_render_zygote->GetSandboxStatus());
  (*GetGenericZygote())->GetSandboxStatus()

https://codereview.chromium.org/1643533006/diff/1/content/browser/zygote_host...
File content/browser/zygote_host/zygote_handle_linux.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/zygote_host...
content/browser/zygote_host/zygote_handle_linux.cc:17: ZygoteHandle
GetGenericZygote() {
Change to:

  ZygoteHandle* GetGenericZygote() {
    static ZygoteHandle zygote;
    return &zygote;
  }

so all of the SandboxedProcessLauncherDelegate::GetZygote() overloads become
something like

  ZygoteHandle* FooDelegate::GetZygote() {
    return content::GetGenericZygote();
  }

https://codereview.chromium.org/1643533006/diff/1/content/public/browser/zygo...
File content/public/browser/zygote_handle_linux.h (right):

https://codereview.chromium.org/1643533006/diff/1/content/public/browser/zygo...
content/public/browser/zygote_handle_linux.h:8: #include <cstddef>
Might as well remove this while you're here.  It's not needed.

[Greg K, 2016-01-29 00:41:04]

https://codereview.chromium.org/1643533006/diff/1/content/browser/browser_mai...
File content/browser/browser_main_loop.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/browser_mai...
content/browser/browser_main_loop.cc:230:
RenderProcessHostImpl::EarlyZygoteLaunch();
On 2016/01/28 23:25:40, mdempsky wrote:
> Before RenderProcessHostImpl::EarlyZygoteLaunch, add
> 
>   *GetGenericZygote() = CreateZygote();

Done.

https://codereview.chromium.org/1643533006/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.cc:553:
g_render_zygote->GetSandboxStatus());
On 2016/01/28 23:25:40, mdempsky wrote:
>   (*GetGenericZygote())->GetSandboxStatus()

Done.

https://codereview.chromium.org/1643533006/diff/1/content/browser/zygote_host...
File content/browser/zygote_host/zygote_handle_linux.cc (right):

https://codereview.chromium.org/1643533006/diff/1/content/browser/zygote_host...
content/browser/zygote_host/zygote_handle_linux.cc:17: ZygoteHandle
GetGenericZygote() {
On 2016/01/28 23:25:40, mdempsky wrote:
> Change to:
> 
>   ZygoteHandle* GetGenericZygote() {
>     static ZygoteHandle zygote;
>     return &zygote;
>   }
> 
> so all of the SandboxedProcessLauncherDelegate::GetZygote() overloads become
> something like
> 
>   ZygoteHandle* FooDelegate::GetZygote() {
>     return content::GetGenericZygote();
>   }

Done.

https://codereview.chromium.org/1643533006/diff/1/content/public/browser/zygo...
File content/public/browser/zygote_handle_linux.h (right):

https://codereview.chromium.org/1643533006/diff/1/content/public/browser/zygo...
content/public/browser/zygote_handle_linux.h:8: #include <cstddef>
On 2016/01/28 23:25:40, mdempsky wrote:
> Might as well remove this while you're here.  It's not needed.

Done.

[mdempsky, 2016-01-29 01:09:59]

lgtm

[Greg K, 2016-01-29 01:12:55]

kerrnel@chromium.org changed reviewers:
+ jochen@chromium.org

[Greg K, 2016-01-29 01:12:56]

jochen@chromium.org, would you please provide an OWNERS review? This is designed
to temporarily go back to a single global zygote while performance issues are
addressed.

Thanks,

Greg

[jochen (gone - plz use gerrit), 2016-01-29 09:19:58]

lgtm

[Greg K, 2016-02-01 17:49:56]

The CQ bit was checked by kerrnel@chromium.org

[Greg K, 2016-02-01 17:49:56]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
mdempsky@chromium.org
Link to the patchset: https://codereview.chromium.org/1643533006/#ps40001
(title: "Remove unused ppapi header")

[commit-bot: I haz the power, 2016-02-01 17:50:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643533006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643533006/40001

[commit-bot: I haz the power, 2016-02-01 18:57:36]

Description was changed from

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974
==========

to

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974
==========

[commit-bot: I haz the power, 2016-02-01 18:57:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-01 18:58:33]

Description was changed from

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974
==========

to

==========
Use a single, generic, global zygote to temporarily resolve perf regressions.

The CL to have multiple zygotes has caused some performance regressions,
due to the additional startup time required to launch the processes. This CL
keeps then new API in place, but uses a single global zygote, resolving the
startup overhead. Once the performance of permanent solutions has been measured
and thought through, Chrome will return to having multiple zygote processes.

BUG=569191,580966,580974

Committed: https://crrev.com/94807ee60fa56c118cac88b0f8f4e4e5231ef43f
Cr-Commit-Position: refs/heads/master@{#372709}
==========

[commit-bot: I haz the power, 2016-02-01 18:58:35]

Patchset 3 (id:??) landed as
https://crrev.com/94807ee60fa56c118cac88b0f8f4e4e5231ef43f
Cr-Commit-Position: refs/heads/master@{#372709}