Closed Bug 1589122 Opened 3 months ago Closed 3 months ago

The interval in a new profile is 0.001ms but it should be 1ms.

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)

defect

Tracking

(firefox-esr68 unaffected, firefox67 unaffected, firefox68 unaffected, firefox69 unaffected, firefox70 unaffected, firefox71 fixed, firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: julienw, Assigned: julienw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(8 files)

Attached image screenshot

STR:

  1. open a firefox with a new profile.
  2. enable the profiler icon from the menu Tools > Web Developer > Enable Profiler Toolbar Icon
  3. click the profiler icon

=> notice the interval is set at 0.001ms + the overhead is fully red.

The same happens in the experimental devtools panel.

This comes from a change in bug 1587117 where the default is supposed to be 1000 (usec) but is 1.

This also renames various variables from "settings" to "preferences" to
make it clearer that the values are about actual preferences stored in
the user profile.

This value isn't really used, nevertheless it's good to have it to the
right value for documentation reason and consistency.

Regressed by: 1587117
No longer regressed by: 1588189
Pushed by jwajsberg@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/909df9883d38
Rename getDefaultRecordingSettings to getDefaultRecordingPreferences r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/a99e9fb9591b
Appropriately configure the default interval pref in microseconds r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/59a3aeee924e
Fix the default interval in reducers r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/acee023cadca
Add comments to the functions called when using the shortcut keys r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/7cdd7440a5b3
Add and use functions to translate values between the stored preferences and the state r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/0e091e3a56e5
Rename some arguments from settings to prefs where appropriate r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/217dd3192072
Add a simple check to the xpcshell test r=canaltinova

Comment on attachment 9101945 [details]
Bug 1589122 - Rename getDefaultRecordingSettings to getDefaultRecordingPreferences

Beta/Release Uplift Approval Request

  • User impact if declined: New profiles will have a bad default value as the "interval" when profiling, which makes firefox very slow (nearly unresponsive) if it's not changed.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is low risk mainly because this code isn't exposed to end-users but rather to gecko developers. Even for gecko developers it requires enabling a pref.

Several alternatives:

  • a simpler patch is probably possible, but because this needs also some manual testing in addition to the automated testing I'd rather use the same patchset.
  • not uplifting.

I'd still rather uplift because otherwise the behavior could be surprising if for some reason a gecko developers would want to use the internal profiler popup on a new profile.

  • String changes made/needed:
Attachment #9101945 - Flags: approval-mozilla-beta?
Attachment #9101946 - Flags: approval-mozilla-beta?
Attachment #9101947 - Flags: approval-mozilla-beta?
Attachment #9101948 - Flags: approval-mozilla-beta?
Attachment #9101949 - Flags: approval-mozilla-beta?
Attachment #9101950 - Flags: approval-mozilla-beta?
Attachment #9102527 - Flags: approval-mozilla-beta?

Comment on attachment 9101945 [details]
Bug 1589122 - Rename getDefaultRecordingSettings to getDefaultRecordingPreferences

Big patch but has tests and impacts only gecko developers, so let's take it in 71 beta 4 as we are just starting the beta cycle, thanks.

Attachment #9101945 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9101946 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9101947 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9101948 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9101949 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9101950 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9102527 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.