Improve instant process performance

[Wangsong Jin]

Hi folks,

Our team has been working on improving the performance of the Instant process to enhance the start-up time for third-party NTPs by utilizing the existing spare renderer. Doc: Improving instant process creation performance 

Crbug: Improve instant process performance [398159675]

This work is mainly passing the instant-process flag via WebPreferences instead of command-line. WebPreferences is passed while the RenderFrame gets created so that we can do late-bind and change the currency then Spare Renderer can be used, since Spare Renderer has no RenderFrame yet.

We're also working out some details with site isolation. For example, we might still prevent pages from the same site but with different origins from sharing the same (spare) process with NTPs that have specified instant process-related APIs.

As a starting point, we're reaching out to the navigation-dev@ team for suggestions on the idea of the late-bind instant process flag part. I greatly appreciate your time and assistance with this!

[Wangsong Jin]

Hi folks,

Our team has been working on improving the performance of the Instant process to enhance the start-up time for third-party NTPs by utilizing the existing spare renderer. 

Doc: Improving instant process creation performance - Google Docs 

Crbug: Improve instant process performance [398159675] - Chromium

This task is mainly about passing the instant-process flag via WebPreferences instead of command-line. WebPreferences are applied while the RenderFrame gets created,  so that we can do late-bind and change the currency then spare renderer can be used, since spare Renderer has no RenderFrame yet.

We're also working out some details with site isolation. With the change to using the existing spare renderer, we want to ensure that other pages (for example same site but different origin) don't share the same process with NTPs that have specified instant process-related APIs.

As a starting point, we're reaching out to the navigation-dev@ team for suggestions on the idea of the late-binding instant process flag. I greatly appreciate your time and assistance with this!

[Alex Moshchuk]

Overall, removing the instant-process command-line flag and migrating to something in WebPreferences seems like a good idea. I'm not really familiar with how the flag is used today, but I'm assuming it's not going to be needed before any RenderFrames are created in the renderer? There's one use of it which appears to be on renderer process startup - presumably that one will need to change somehow?

Note that another way to support this could be to maintain multiple spare processes with potentially different cmdline flags. There's some work happening in that direction (e.g., https://crbug.com/364635886), but if we can just remove the instant-process flag and not treat the instant process as special, that's even better.

Thanks,

Alex

--
You received this message because you are subscribed to the Google Groups "navigation-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to navigation-de...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/7145dfd2-4589-44e6-9d61-7a8a8a5e9f93n%40chromium.org.

[Wangsong Jin]

Hi Alex,

Thank you for your response! I'm happy to hear you like the idea.

> "I'm assuming it's not going to be needed before any RenderFrames are created in the renderer"

Yes, the Instant process is used to expose the SearchBox API, which is not installed during process creation. Instead, it is created in RenderFrameCreated() and then installed as part of the navigation process.

(We can do WebPreferences instant process flag check in where SearchBox is Created and Installed).

 

We're in the process prototyping, and the late-binding instant process flag part works. However, we need to address some site-isolation issues with the change. Once we finalize the prototype, we will confirm with you and site-isolation team that it's something we can ship into Chromium.  Please feel free to let me know if you have any other suggestions or questions.

Thanks,

Wangsong

[Wangsong Jin]

Hi Alex,

 
We've completed the prototype. Here are the implementation details:
Add a Feature Flag

• Add a feature flag kInstantUsesSpareRenderer in the /components/ntp folder. 

Pass Instant via WebPreferences Instead of a Command-Line Switch

1. Add a new WebPreferences parameter enable_instant_api in web_preferences.mojom, and update the corresponding files:  

• /third_party/blink/public/common/web_preferences/web_preferences.h
• /third_party/blink/public/common/web_preferences/web_preferences_mojom_traits.h
• /third_party/blink/common/web_preferences/web_preferences_mojom_traits.cc

2. Add the FeatureList::IsEnabled(ntp::features::kInstantUsesSpareRenderer) check in ChromeContentBrowserClient::AppendExtraCommandLineSwitches() to skip the --instant-process command line.
3. Set the enable_instant_api parameter value if it's an instant URL and the kInstantUsesSpareRenderer flag is enabled in ChromeContentBrowserClient::OverrideWebPreferences.
4. Allow later installation and creation of the search box when enable_instant_api is enabled in ChromeRenderFrameObserver::DidClearWindowObject and ChromeContentRendererClient::RenderFrameCreated. Also, move WebSecurityPolicy::RegisterURLSchemeAsDisplayIsolated(chrome_search_scheme) to a later call in ChromeContentRendererClient::RenderFrameCreated.

Enable third party NTPs to use spare renderer

1. Bypass the refusal to use the renderer process if ntp::features::kInstantUsesSpareRenderer is enabled inChromeContentBrowserClient::ShouldUseSpareRenderProcessHost.
2. Skip the initial IsInstantCheck for Instant URLs using the spare renderer. InstantService::AddInstantProcess(content::RenderProcessHost* host) is called after content::SiteInstanceImpl::GetOrCreateProcess(). As a result, the first spare process doesn't get a chance to add the instant process ID, but RenderProcessHostImpl::MayReuseAndIsSuitable is called, hitting the NOTREACHED().

Add a new field (spare_was_taken = false) in RenderProcessHostImpl::MayReuseAndIsSuitable, RenderProcessHostImpl::IsSuitableHost, and ChromeContentBrowserClient::IsSuitableHost.
• Update ChromeContentBrowserClient::IsSuitableHost to return true if it's an instant URL and the spare render was taken.
  Add a getter SiteInstanceProcessAssignment::process_assignment() so we can pass the spare was taken in RenderProcessHostImpl::MayReuseAndIsSuitable.

 

With this change, we continue to adhere to the Process-Per-Site mode for third-party NTPs, ensuring there are no site-isolation violations.

• Existing Behavior: The first third-party NTP will create a process, and subsequent same-site third-party NTPs will share this process.
• New Behavior: The first third-party NTP will attempt to use a spare process if available. If a spare process exists, it will be used, and subsequent same-site third-party NTPs will reuse this spare process. If no spare process is available, it will follow the existing behavior by creating a new process and reusing it for subsequent same-site NTPs.

If you think those are good. Next step, I will create a CL for you and site-isolation team to review.

Additionally, we are curious about how to test this change in the wild before fully enabling it. What is your typical flighting/rollout plan? Would you be able to run the experiment on your end?

Thanks!

[Alex Moshchuk]

HI Wangsong,

These changes generally sound reasonable. I'm not sure about a couple of them (e.g., why is it necessary to change IsSuitableHost), but this is going to be easier to discuss in a code review, so please feel free to proceed with putting together a CL for this.

For this question:

Additionally, we are curious about how to test this change in the wild before fully enabling it. What is your typical flighting/rollout plan? Would you be able to run the experiment on your end?

Depending on the change, we either roll it out via field trials (running A/B testing while progressing through canary/dev/beta/stable) or if we don't need to necessarily measure the impact, we just enable the new feature by default and let it trickle through all channels on Chrome's regular release schedule, and if anything goes wrong, the base::Feature can be used as a kill switch.

We'd prefer if you roll out this feature on your end. :) I think I've heard that Edge has an A/B platform similar to Finch? So maybe one option is to use that and possibly share high-level results with us before flipping it on? Though unless you're interested in measuring the perf improvement from this change, it could also be ok to just make the change (while keeping the feature around as a kill switch). It's pretty clear that it's a desirable change, and since it only affects third-party NTPs, I'm guessing the aggregate metrics we typically look at probably won't really show anything interesting anyway.

Thanks,

Alex  

To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/1ecd24fe-12f2-4306-97e1-177556cf7174n%40chromium.org.

[Wangsong Jin]

Hi Alex,

Thanks for your clarification! We'll creating the CL. Regarding the experimental part, Edge has an A/B testing platform, so we'll land the change, run the experiment on Edge, share performance results to you, and then flip the switch on Chromium.

Thanks,

Wangsong

[Wangsong Jin]

Hi Alex, 

Apologies for the delayed response. It took some time to get the prototype ready for the review phase. We have changed where the instant process flag is passed (updated the doc)

Here're two CLs:

• Improve instant process creation performance by using spare renderer (6373046) · Gerrit Code Review
• Refractor maybeTakeSpare to short-circuits all the checks in the order they are done (6374610) · Gerrit Code Review

Please take a look when you have the change, thanks!

Thank you,
Wangsong