Last Comment Bug 727864 - Enable DLL preloading when MozillaMaintenance service disabled prefetch
: Enable DLL preloading when MozillaMaintenance service disabled prefetch
Status: VERIFIED FIXED
[qa?]
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal with 1 vote (vote)
: mozilla15
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
Depends on: 692255 758206
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-16 08:51 PST by Brian R. Bondy [:bbondy]
Modified: 2012-08-27 13:21 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1. (6.38 KB, patch)
2012-02-16 09:31 PST, Brian R. Bondy [:bbondy]
no flags Details | Diff | Review
Patch v2. (6.48 KB, patch)
2012-02-17 07:32 PST, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Review
Patch v3. (6.49 KB, patch)
2012-02-22 07:29 PST, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Review
Patch v4 (6.49 KB, patch)
2012-05-14 09:35 PDT, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Review

Description Brian R. Bondy [:bbondy] 2012-02-16 08:51:56 PST
We currently only do DLL preloading when we think that the Windows prefetch isn't used.  We should also do DLL preloading when the maintenance service has performed a successful clear prefetch operation.
Comment 1 Brian R. Bondy [:bbondy] 2012-02-16 09:31:57 PST
Created attachment 597866 [details] [diff] [review]
Patch v1.
Comment 2 Brian R. Bondy [:bbondy] 2012-02-17 07:30:52 PST
Alfred Kayser 2012-02-16 23:59:42 PST

I would move the LOG statement of the failing WritePrefetchClearedReg function inside that function, just as the other LOG statement is also there.
+  if (retCode != ERROR_SUCCESS) {
+    LOG(("Could not open key. (%d)\n", retCode));
+  }
And, add a 'return false' here after the LOG.
Comment 3 Brian R. Bondy [:bbondy] 2012-02-17 07:32:02 PST
Created attachment 598233 [details] [diff] [review]
Patch v2.

Implemented review comments and rebased to m-c tip.
Comment 4 Jim Mathies [:jimm] 2012-02-21 07:48:04 PST
Comment on attachment 598233 [details] [diff] [review]
Patch v2.

Review of attachment 598233 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/nsBrowserApp.cpp
@@ +251,5 @@
>    // in the program. Luckily 1 coincides with when prefetch is
>    // enabled. If Windows prefetch didn't happen we can do our own
>    // faster dll preloading.
> +  // The MozillaMaintenance service issues a command to disable the
> +  // pretch by replacing all found .pf files with 0 byte read only

nit - 'pretch'

::: toolkit/components/maintenanceservice/prefetch.cpp
@@ +54,5 @@
> +  LONG retCode = RegOpenKeyExW(HKEY_LOCAL_MACHINE, 
> +                               BASE_SERVICE_REG_KEY, 0,
> +                               KEY_WRITE | KEY_WOW64_64KEY, &baseKeyRaw);
> +  if (retCode != ERROR_SUCCESS) {
> +    LOG(("Could not open key. (%d)\n", retCode));

does this LOG macro do file name / line number logging? If not you might want to make that msg more specific.
Comment 5 Brian R. Bondy [:bbondy] 2012-02-22 07:29:40 PST
Created attachment 599602 [details] [diff] [review]
Patch v3.

Implemented nits, carrying forward r+.
Comment 6 Brian R. Bondy [:bbondy] 2012-02-22 07:30:33 PST
Comment on attachment 599602 [details] [diff] [review]
Patch v3.

Actually I don't think this one needs a super-review so clearing that flag.
Comment 7 Brian R. Bondy [:bbondy] 2012-05-14 09:35:42 PDT
Created attachment 623704 [details] [diff] [review]
Patch v4

Rebased to tip
Comment 8 Brian R. Bondy [:bbondy] 2012-05-27 19:49:59 PDT
http://hg.mozilla.org/mozilla-central/rev/21ae85ff4286
Comment 9 Jason Smith [:jsmith] 2012-06-22 16:19:36 PDT
Is there anything to verify here from an end-user perspective?
Comment 10 Brian R. Bondy [:bbondy] 2012-06-22 17:34:32 PDT
No this was verified by seeing telemetry data.

Note You need to log in before you can comment on or make changes to this bug.