Closed
Bug 1339327
Opened 7 years ago
Closed 7 years ago
A bunch more Profiler clean-ups
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(8 files)
2.00 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
7.47 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
2.47 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
41.67 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
10.44 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
3.88 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
9.89 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
2.78 KB,
patch
|
jseward
:
review+
|
Details | Diff | Splinter Review |
I have a bunch more profiler clean-ups.
Assignee | ||
Comment 1•7 years ago
|
||
"unwinder" and "jank" are never checked for via hasFeature(), and are not mentioned anywhere else in the profiler. This patch removes them from profiler_get_features(). "restyle" *does* have a hasFeature() call and is handled by profiler_feature_active(). This patch adds it to profiler_get_features().
Attachment #8836991 -
Flags: review?(mstange)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
This patch: - Improves some comments. - Adds some assertions. - Removes unnecessary differences between some Mac and Windows code. - Removes the unnecessary SamplerThread::mStackSize on Windows.
Attachment #8836992 -
Flags: review?(mstange)
Assignee | ||
Comment 3•7 years ago
|
||
This is possible because profiler_shutdown() can call profiler_save_profile_to_file() to do the saving.
Attachment #8836993 -
Flags: review?(mstange)
Assignee | ||
Comment 4•7 years ago
|
||
The 'g' prefix is for global variables. The 's' prefix is for static class members.
Attachment #8836994 -
Flags: review?(mstange)
Assignee | ||
Comment 5•7 years ago
|
||
The new names make it clearer that these actions apply to just one thread. - profiler_sleep_start() --> profiler_thread_sleep() - profiler_sleep_end() --> profiler_thread_wake() - profiler_is_sleeping() --> profiler_thread_is_sleeping() - GeckoProfilerSleepRAII --> GeckoProfilerThreadSleepRAII - GeckoProfilerWakeRAII --> GeckoProfilerThreadWakeRAII
Attachment #8836995 -
Flags: review?(mstange)
Assignee | ||
Comment 6•7 years ago
|
||
It's entirely unused, except for DISALLOW_COPY_AND_ASSIGN, which is easy to replace.
Attachment #8836997 -
Flags: review?(mstange)
Assignee | ||
Comment 7•7 years ago
|
||
The new names are more consistent, and I find them clearer. The patch also inlines and removes the horribly-named ProfilerSignalThread() function.
Attachment #8836998 -
Flags: review?(mstange)
Assignee | ||
Comment 8•7 years ago
|
||
Instead of calling MaybeShowStats() every 16th time around the inner loop we now check every time around the outer loop. Because there are typically 20--50 threads running at once, this results in a slightly lower freqency of printing, but that seems fine because this is debug-only code.
Attachment #8837000 -
Flags: review?(jseward)
Updated•7 years ago
|
Attachment #8837000 -
Flags: review?(jseward) → review+
Updated•7 years ago
|
Attachment #8836991 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836992 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836993 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836994 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836995 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836997 -
Flags: review?(mstange) → review+
Updated•7 years ago
|
Attachment #8836998 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 9•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5da29c4aafb01e308dde13260adc8edf1ab2bf65 Bug 1339327 (part 1) - Fix profiler_get_features(). r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/bc1c0da509e47dcd9907badd5c9fde1a3356f95f Bug 1339327 (part 2) - Tweak platform-*.cc files. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/5cb6c0a02b7e491cafef68ba782791ebdde8b878 Bug 1339327 (part 3) - Remove ToStreamAsJSON(). r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/c6933c5687aaee7f3c261364f077b4d1fa4d3d8a Bug 1339327 (part 4) - Rename some globals in the profiler. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa60b9dbadcc1352193a4b64367ed2135dd384d Bug 1339327 (part 5) - Rename some profiler functions related to sleeping. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c8118d95f32d06261d69317e3d14f1b4fd9648 Bug 1339327 (part 6) - Remove v8-support.h. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/542fffc2c125254f39a8457abfb69671269c6dd1 Bug 1339327 (part 7) - Rename some things in platform-linux.cc. r=mstange. https://hg.mozilla.org/integration/mozilla-inbound/rev/e538c3740be131f78a98129221f16d4dacb29906 Bug 1339327 (part 8) - Move the LUL printing check out of SignalSender()'s inner loop. r=jseward.
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5da29c4aafb0 https://hg.mozilla.org/mozilla-central/rev/bc1c0da509e4 https://hg.mozilla.org/mozilla-central/rev/5cb6c0a02b7e https://hg.mozilla.org/mozilla-central/rev/c6933c5687aa https://hg.mozilla.org/mozilla-central/rev/1aa60b9dbadc https://hg.mozilla.org/mozilla-central/rev/d6c8118d95f3 https://hg.mozilla.org/mozilla-central/rev/542fffc2c125 https://hg.mozilla.org/mozilla-central/rev/e538c3740be1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•