Chromium Code Reviews
Help | Chromium Project | Sign in
(242)

Issue 2936010: Boot prefetch optimization for Chrome Frame (experimental)... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 10 months ago by amit
Modified:
3 years, 5 months ago
Reviewers:
slightlyoff, tommi
CC:
chromium-reviews, amit
Visibility:
Public.

Description

Boot prefetch optimization for Chrome Frame (experimental) If chrome is warmed up during a single reboot, it gets paged in for subsequent reboots and the cold startup times essentially look like warm times thereafter! The 'warm up' is done by setting up a 'RunOnce' key during DLLRegisterServer of npchrome_frame.dll. This works because chrome prefetch becomes part of boot prefetch file ntosboot-b00dfaad.pf and paged in on subsequent reboots. As long as the sytem does not undergo significant memory pressure those pages remain in memory and we get pretty amazing startup times, down to about 300 ms from 1200 ms The downside is: - Whether chrome frame is used or not, there's a read penalty (1200-300 =) 900 ms for every boot. - Heavy system memory usage after reboot will nullify the benefits but the user will still pay the cost. - Overall the time saved will always be less than total time spent paging in chrome - We are not sure when the chrome 'warm up' will age out from the boot prefetch file. The idea here is to try this out on chrome frame dev channel and see if it produces a significant drift in startup numbers. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52489

Patch Set 1 #

Total comments: 7

Patch Set 2 : '' #

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -45 lines) Patch
M chrome_frame/chrome_tab.cc View 1 2 7 chunks +49 lines, -45 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 5 (0 generated)
amit
4 years, 10 months ago (2010-07-12 23:44:19 UTC) #1
slightlyoff
Thanks for writing up our research so clearly. http://codereview.chromium.org/2936010/diff/1/2 File chrome_frame/chrome_tab.cc (right): http://codereview.chromium.org/2936010/diff/1/2#newcode306 chrome_frame/chrome_tab.cc:306: if ...
4 years, 10 months ago (2010-07-12 23:49:36 UTC) #2
tommi
lgtm with alex' comments http://codereview.chromium.org/2936010/diff/1/2 File chrome_frame/chrome_tab.cc (right): http://codereview.chromium.org/2936010/diff/1/2#newcode301 chrome_frame/chrome_tab.cc:301: HRESULT SetupRunOnce() { Can you ...
4 years, 10 months ago (2010-07-13 15:04:15 UTC) #3
amit
http://codereview.chromium.org/2936010/diff/1/2 File chrome_frame/chrome_tab.cc (right): http://codereview.chromium.org/2936010/diff/1/2#newcode306 chrome_frame/chrome_tab.cc:306: if (!GoogleUpdateSettings::GetChromeChannel(true, &channel_name) || On 2010/07/12 23:49:36, slightlyoff wrote: ...
4 years, 10 months ago (2010-07-14 22:06:32 UTC) #4
slightlyoff
4 years, 10 months ago (2010-07-15 01:04:40 UTC) #5
LGTM

On 2010/07/14 22:06:32, amit wrote:
> http://codereview.chromium.org/2936010/diff/1/2
> File chrome_frame/chrome_tab.cc (right):
> 
> http://codereview.chromium.org/2936010/diff/1/2#newcode306
> chrome_frame/chrome_tab.cc:306: if
> (!GoogleUpdateSettings::GetChromeChannel(true, &channel_name) ||
> On 2010/07/12 23:49:36, slightlyoff wrote:
> > This check is unnecessary. If it doesn't pan out on Dev, we can just remove
> > this.
> Belt and suspenders :) 
> Branch date is near and we might need to run this for a few weeks to get a
> meaningful feedback. I don't want this to be enabled for anything else but
dev.
> 
> http://codereview.chromium.org/2936010/diff/1/2#newcode312
> chrome_frame/chrome_tab.cc:312: if (run_once.Open(HKEY_LOCAL_MACHINE,
kRunOnce,
> KEY_READ | KEY_WRITE)) {
> On 2010/07/12 23:49:36, slightlyoff wrote:
> > why not HKCU?
> 
> Done.
> 
> http://codereview.chromium.org/2936010/diff/1/2#newcode315
> chrome_frame/chrome_tab.cc:315: switches::kAutomationClientChannelID,
_T("0"));
> On 2010/07/13 15:04:16, tommi wrote:
> > L instead of _T()?
> 
> Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be