If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Cleanup profiler headers

RESOLVED FIXED in mozilla22

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

Trunk
mozilla22
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 725532 [details] [diff] [review]
Part 1: Change interface
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 725533 [details] [diff] [review]
Part 2: Change calls
(Assignee)

Comment 3

5 years ago
Created attachment 725534 [details] [diff] [review]
Part 3: Rename headers
(Assignee)

Comment 4

5 years ago
Created attachment 725535 [details] [diff] [review]
Part 1: Change interface
Attachment #725532 - Attachment is obsolete: true
(Assignee)

Comment 5

5 years ago
Comment on attachment 725534 [details] [diff] [review]
Part 3: Rename headers

Generated using:
find . -type f -name "*.cpp" | xargs grep -l \"sampler.h\" | xargs perl -pi -e 's/\"sampler.h\"/\"GeckoProfiler.h\"/g'
(copied for .h and .mm)
(Assignee)

Updated

5 years ago
Attachment #725535 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Attachment #725533 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Attachment #725534 - Flags: review?(ehsan)
(Assignee)

Comment 6

5 years ago
Created attachment 725558 [details] [diff] [review]
Part 3: Rename headers (build fix)
Attachment #725534 - Attachment is obsolete: true
Attachment #725534 - Flags: review?(ehsan)
Attachment #725558 - Flags: review?(ehsan)
Attachment #725533 - Flags: review?(ehsan) → review+
Attachment #725535 - Flags: review?(ehsan) → review+
Attachment #725558 - Flags: review?(ehsan) → review+
(Assignee)

Updated

5 years ago
Attachment #725558 - Attachment is patch: true
(Assignee)

Updated

5 years ago
Blocks: 851748
(Assignee)

Comment 7

5 years ago
Created attachment 725682 [details] [diff] [review]
Part 4: Split out GeckoProfilerFunc.h & PseudoStack.h

I only need one review. But I'd love to land this by Monday for snappy.
Attachment #725682 - Flags: review?(jmuizelaar)
Attachment #725682 - Flags: review?(ehsan)
Attachment #725682 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

5 years ago
Attachment #725682 - Flags: review?(ehsan)
(Assignee)

Comment 8

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=c815ae6156d0
(Assignee)

Comment 9

5 years ago
Created attachment 725715 [details] [diff] [review]
Part 2: Change calls. r=jmuizel

Fixed linux build failure
Attachment #725533 - Attachment is obsolete: true
Attachment #725715 - Flags: review+
(Assignee)

Comment 10

5 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/77f321ed3d9e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c75481a07302
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d195190adc48
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ecce21507ea0
Backed out for breaking the build:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ecce21507ea0

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/74795c6fd7d6
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/be8691daf71e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1bed4fd2b0fa
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e23e43a2c14e
(Assignee)

Comment 12

5 years ago
*sigh* It was just a conflict that wasn't properly resolved in something that changes. These patches are changing a lot so its going to hit a lot of rot.

Pushing to try just to be safe but this should be fine:
https://tbpl.mozilla.org/?tree=Try&rev=a0a6f3bf555b
(Assignee)

Comment 13

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=35a99b0db13b
(Assignee)

Comment 14

5 years ago
Someone accidentally canceled the previous push:
https://tbpl.mozilla.org/?tree=Try&rev=edfb1e65e9e5
(Assignee)

Comment 15

5 years ago
Try reset. Repush:
https://tbpl.mozilla.org/?tree=Try&rev=edfb1e65e9e5
(Assignee)

Comment 16

5 years ago
Ok after repushing my data started showup again. Assuming it's the data for the right push I'm putting this on inbound.
(Assignee)

Comment 17

5 years ago
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/99e09a7e03e6
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7b508d11a791
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bf04a3230bfe
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a428deb3fa9a
Created attachment 729203 [details] [diff] [review]
Followup : fix platforms without sps profiler

I think this broke platforms where MOZ_ENABLE_PROFILER_SPS is undef..

18:30.27 In file included from /src/mozilla-central/xpcom/base/nsCycleCollector.cpp:129:
18:30.27 ../../dist/include/GeckoProfiler.h:107:51: error: unknown type name 'TimeStamp'; did you mean 'mozilla::TimeStamp'?
18:30.27 static inline void profiler_responsinveness(const TimeStamp& aTime) {}
18:30.27                                                   ^~~~~~~~~
18:30.27                                                   mozilla::TimeStamp
18:30.27 ../../dist/include/mozilla/TimeStamp.h:204:7: note: 'mozilla::TimeStamp' declared here
18:30.27 class TimeStamp
18:30.27       ^

Trying the obvious fixes it here.
Attachment #729203 - Flags: review?(bgirard)
Hm, spoke too fast, more might be needed :

18:18.18 /src/mozilla-central/gfx/layers/opengl/LayerManagerOGL.cpp:934:3: error: use of undeclared identifier 'profiler_set_frame_number'; did you mean 'profile_set_frame_number'?
18:18.18   profiler_set_frame_number(sFrameCount);
18:18.18   ^~~~~~~~~~~~~~~~~~~~~~~~~
18:18.18   profile_set_frame_number
18:18.18 ../../dist/include/GeckoProfiler.h:113:20: note: 'profile_set_frame_number' declared here
18:18.18 static inline void profile_set_frame_number(int frameNumber) {}
(Assignee)

Comment 20

5 years ago
Comment on attachment 729203 [details] [diff] [review]
Followup : fix platforms without sps profiler

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

Feel free to also ride along s/profile_set_frame_number/profiler_set_frame_number/g. Thanks for fixing!
Attachment #729203 - Flags: review?(bgirard) → review+
https://hg.mozilla.org/mozilla-central/rev/99e09a7e03e6
https://hg.mozilla.org/mozilla-central/rev/7b508d11a791
https://hg.mozilla.org/mozilla-central/rev/bf04a3230bfe
https://hg.mozilla.org/mozilla-central/rev/a428deb3fa9a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
There were 3 actual typos : profiler_responsinveness -> profiler_responsiveness, profile_set_frame_number -> profiler_set_frame_number and the missing mozilla::.

https://hg.mozilla.org/integration/mozilla-inbound/rev/795b10c2a7f4

Reopening for mergetool.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/795b10c2a7f4
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 856281
You need to log in before you can comment on or make changes to this bug.