Closed
Bug 851828
Opened 11 years ago
Closed 11 years ago
Type safety improvements for the profiler in Win64 builds
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file, 1 obsolete file)
5.49 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #725795 -
Flags: review?(jmuizelaar)
Attachment #725795 -
Flags: review?(bgirard)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → ehsan
Comment 1•11 years ago
|
||
Comment on attachment 725795 [details] [diff] [review] Patch (v1) Review of attachment 725795 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/sps_sampler.h @@ +472,5 @@ > return mStackPointer == 0; > } > uint32_t stackSize() const > { > + return std::min(mStackPointer, mozilla::sig_safe_t(mozilla::ArrayLength(mStack))); Why did you drop <uint32_t>? The return type is still that.
Attachment #725795 -
Flags: review?(bgirard) → review+
Comment 3•11 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #2) > Please rebase on top of bug 851748 Err I meant this should land on top of bug 851748, not that you have to do it. I can rebase+land this for you when I land bug 851748.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to comment #3) > (In reply to Benoit Girard (:BenWa) from comment #2) > > Please rebase on top of bug 851748 > > Err I meant this should land on top of bug 851748, not that you have to do it. > I can rebase+land this for you when I land bug 851748. That would be great, thanks.
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to comment #1) > Comment on attachment 725795 [details] [diff] [review] > --> https://bugzilla.mozilla.org/attachment.cgi?id=725795 > Patch (v1) > > Review of attachment 725795 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: tools/profiler/sps_sampler.h > @@ +472,5 @@ > > return mStackPointer == 0; > > } > > uint32_t stackSize() const > > { > > + return std::min(mStackPointer, mozilla::sig_safe_t(mozilla::ArrayLength(mStack))); > > Why did you drop <uint32_t>? The return type is still that. unsigned long and uint32_t are interchangeable... But I can add it back if you want.
Comment 7•11 years ago
|
||
Going to try and land this today.
Assignee: ehsan → bgirard
Flags: needinfo?(bgirard)
Comment 8•11 years ago
|
||
Attachment #725795 -
Attachment is obsolete: true
Attachment #725795 -
Flags: review?(jmuizelaar)
Attachment #729627 -
Flags: review+
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5a256c02fc6
Assignee: bgirard → ehsan
Comment 10•11 years ago
|
||
Backout for Mochitest failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/c9ac624cfc84
Comment 11•11 years ago
|
||
This ended up being a red-herring. Re-landed. https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5c83ed0385
Comment 12•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #11) > This ended up being a red-herring. Re-landed. > https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5c83ed0385 Thank you :)
Comment 13•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0c5c83ed0385
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•