Prerender: Disable kPrerender2MemoryControls by default on Android except WebView [chromium/src : main]
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Commit-Queue | +1 |
1 comment
Hi, can you review this CL?
+boilu@ for android_webview/
+lingqi@ for the entire changes.
Thanks!
Related details
- Bo Liu
- Lingqi Chi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Lingqi Chi (Gerrit)
Lingqi Chi voted Code-Review+1![]( "Open in Gerrit")
- Bo Liu
- Hiroki Nakagawa
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bo Liu (Gerrit)
Bo Liu added 1 comment![]( "Open in Gerrit")
AwFieldTrials. The Android WebView launch will be handled separatelyI don't understand this bit.. if it's already enabled by default, then what's left to launch?
Related details
- Hiroki Nakagawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa voted and added 2 comments![]( "Open in Gerrit")
Votes added by Hiroki Nakagawa
| Auto-Submit | +1 |
2 comments
AwFieldTrials. The Android WebView launch will be handled separatelyI don't understand this bit.. if it's already enabled by default, then what's left to launch?
The purpose of this change is to **DISABLE** the memory controls on Android and WebView by default in order to address recent regressions caused by sensitive memory controls.
Related details
- Bo Liu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bo Liu (Gerrit)
Bo Liu added 1 comment![]( "Open in Gerrit")
AwFieldTrials. The Android WebView launch will be handled separatelyHiroki NakagawaI don't understand this bit.. if it's already enabled by default, then what's left to launch?
The purpose of this change is to **DISABLE** the memory controls on Android and WebView by default in order to address recent regressions caused by sensitive memory controls.
- Hiroki Nakagawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
AwFieldTrials. The Android WebView launch will be handled separatelyHiroki NakagawaI don't understand this bit.. if it's already enabled by default, then what's left to launch?
The purpose of this change is to **DISABLE** the memory controls on Android and WebView by default in order to address recent regressions caused by sensitive memory controls.
This is because we're ramping up the launch experiments for Android and WebView separately (`Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView`). The Android experiment has already reached the stable channel, but the WebView experiment hasn't reached it yet. Once it reaches stable as well, I'll disable it for WebView by default and then push the experiment to "launched".
Related details
- Bo Liu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bo Liu (Gerrit)
Bo Liu added 1 comment![]( "Open in Gerrit")
AwFieldTrials. The Android WebView launch will be handled separatelyHiroki NakagawaI don't understand this bit.. if it's already enabled by default, then what's left to launch?
Hiroki NakagawaThe purpose of this change is to **DISABLE** the memory controls on Android and WebView by default in order to address recent regressions caused by sensitive memory controls.
This is because we're ramping up the launch experiments for Android and WebView separately (`Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView`). The Android experiment has already reached the stable channel, but the WebView experiment hasn't reached it yet. Once it reaches stable as well, I'll disable it for WebView by default and then push the experiment to "launched".
This is because we're ramping up the launch experiments for Android and WebView separately (`Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView`).
Prerender2MemoryControlsWebView doesn't exist yet afaict? (code search is down for me, I just used git grep)
The Android experiment has already reached the stable channel, but the WebView experiment hasn't reached it yet.
Ok, how did webview get to the enabled-by-default state at all? Was that the mistake?
Once it reaches stable as well, I'll disable it for WebView by default and then push the experiment to "launched".
- Hiroki Nakagawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
AwFieldTrials. The Android WebView launch will be handled separatelyHiroki NakagawaI don't understand this bit.. if it's already enabled by default, then what's left to launch?
Hiroki NakagawaThe purpose of this change is to **DISABLE** the memory controls on Android and WebView by default in order to address recent regressions caused by sensitive memory controls.
Bo LiuThis is because we're ramping up the launch experiments for Android and WebView separately (`Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView`). The Android experiment has already reached the stable channel, but the WebView experiment hasn't reached it yet. Once it reaches stable as well, I'll disable it for WebView by default and then push the experiment to "launched".
This is because we're ramping up the launch experiments for Android and WebView separately (`Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView`).
Prerender2MemoryControlsWebView doesn't exist yet afaict? (code search is down for me, I just used git grep)
The Android experiment has already reached the stable channel, but the WebView experiment hasn't reached it yet.
Ok, how did webview get to the enabled-by-default state at all? Was that the mistake?
Once it reaches stable as well, I'll disable it for WebView by default and then push the experiment to "launched".
Prerender2MemoryControlsWebView doesn't exist yet afaict? (code search is down for me, I just used git grep)
The `Prerender2MemoryControlsWebView` experiument has already been running on beta.
Ok, how did webview get to the enabled-by-default state at all? Was that the mistake?
Let me explain the context. Originally, `kPrerender2MemoryControls` was enabled by default on all the platforms as intended (not mistake). This was working well, but recent (non-revertible) changes broke the memory pressure handling only on Android and WebView (see [this internal document](https://docs.google.com/document/d/161aUtMwt3qo2a-7H-rvQ6mXqaV1c3vKRfx4ZA_-bvqs/edit?usp=sharing&resourcekey=0-0TB6ikGPfj7JbA6k8l-a9Q) for details). This breakage is not specific to prerendering, but prerendering has been more significantly impacted by that. This is now treated as an "incident", and we're expected to mitigate it by tentatively disabling the memory pressure (memory controls) for prerendering as soon as possible.
Current setup:
- Desktop: Enabled
- Android: Enabled
- WebView: Enabled
Setup after this CL:
- Desktop: Enabled
- Android: Disabled
- WebView: Enabled
Setup after a follow-up CL for WebView.
- Desktop: Enabled
- Android: Disabled
- WebView: Disabled
Once the breakage is fixed, we'll enable the memory controls for Android and WebView again.
Related details
- Bo Liu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bo Liu (Gerrit)
Bo Liu added 1 comment![]( "Open in Gerrit")
Prerender2MemoryControlsWebView doesn't exist yet afaict? (code search is down for me, I just used git grep)
The `Prerender2MemoryControlsWebView` experiument has already been running on beta.
Oh it's referring to a finch gcl. That's still just using the same Prerender2MemoryControlsAndroid feature in chromium code (which is fine). But if webview is still under experiment and hasn't launched, why is the Prerender2MemoryControlsAndroid feature already enabled by default for webview? Isn't that already wrong?
> Ok, how did webview get to the enabled-by-default state at all? Was that the mistake?Let me explain the context. Originally, `kPrerender2MemoryControls` was enabled by default on all the platforms as intended (not mistake). This was working well, but recent (non-revertible) changes broke the memory pressure handling only on Android and WebView (see [this internal document](https://docs.google.com/document/d/161aUtMwt3qo2a-7H-rvQ6mXqaV1c3vKRfx4ZA_-bvqs/edit?usp=sharing&resourcekey=0-0TB6ikGPfj7JbA6k8l-a9Q) for details). This breakage is not specific to prerendering, but prerendering has been more significantly impacted by that. This is now treated as an "incident", and we're expected to mitigate it by tentatively disabling the memory pressure (memory controls) for prerendering as soon as possible.
Current setup:
- Desktop: Enabled
- Android: Enabled
- WebView: Enabled
Setup after this CL:
- Desktop: Enabled
- Android: Disabled
- WebView: Enabled
Setup after a follow-up CL for WebView.
- Desktop: Enabled
- Android: Disabled
- WebView: Disabled
can I see the follow-up CL? Also why should it be in a follow up instead of just part of this CL?
Once the breakage is fixed, we'll enable the memory controls for Android and WebView again.
- Hiroki Nakagawa
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hiroki Nakagawa (Gerrit)
Hiroki Nakagawa added 1 comment![]( "Open in Gerrit")
Oh it's referring to a finch gcl. That's still just using the same Prerender2MemoryControlsAndroid feature in chromium code (which is fine).
To be clear, `Prerender2MemoryControls` is the feature name to disable/enable the memory controls for prerendering in Chromium code base, and `Prerender2MemoryControlsAndroid` and `Prerender2MemoryControlsWebView` are the experiment names specified in gcl.
But if webview is still under experiment and hasn't launched, why is the Prerender2MemoryControlsAndroid feature already enabled by default for webview? Isn't that already wrong?
Do you mean `Prerender2MemoryControlsAndroid` in `fieldtrial_testing_config.json`? Due to my mistake in the previous CL, `fieldtrial_testing_config.json` refers to the both experiments as `Prerender2MemoryControlsAndroid` but actually there are 2 separate experiments. This CL also fixes it.
can I see the follow-up CL?
I'll make a CL.
Also why should it be in a follow up instead of just part of this CL?
WebView requires more steps to reach the launch ready state. Without waiting for that, we'd like to launch disablement of the memory controls for Android first because the impact on Android is severer than WebView.
Related details
- Bo Liu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |