Closed Bug 851611 Opened 7 years ago Closed 7 years ago

Cleanup profiler headers

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(5 files, 3 obsolete files)

No description provided.
Attached patch Part 1: Change interface (obsolete) — Splinter Review
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attached patch Part 2: Change calls (obsolete) — Splinter Review
Attached patch Part 3: Rename headers (obsolete) — Splinter Review
Attachment #725532 - Attachment is obsolete: true
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)
Attachment #725535 - Flags: review?(ehsan)
Attachment #725533 - Flags: review?(ehsan)
Attachment #725534 - Flags: review?(ehsan)
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+
Attachment #725558 - Attachment is patch: true
Blocks: 851748
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+
Attachment #725682 - Flags: review?(ehsan)
Fixed linux build failure
Attachment #725533 - Attachment is obsolete: true
Attachment #725715 - Flags: review+
*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
Someone accidentally canceled the previous push:
https://tbpl.mozilla.org/?tree=Try&rev=edfb1e65e9e5
Ok after repushing my data started showup again. Assuming it's the data for the right push I'm putting this on inbound.
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) {}
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+
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
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Depends on: 856281
You need to log in before you can comment on or make changes to this bug.