Closed
Bug 1322863
Opened 6 years ago
Closed 6 years ago
use UniquePtr for managing pointers where possible.
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(3 files)
2.24 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
4.07 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
6.21 KB,
patch
|
mstange
:
review+
|
Details | Diff | Splinter Review |
I probably won't catch everything, but I have a patch series to fix a goodly number of places.
![]() |
Assignee | |
Comment 1•6 years ago
|
||
Smart pointers are better than raw pointers.
Attachment #8823387 -
Flags: review?(mstange)
![]() |
Assignee | |
Comment 2•6 years ago
|
||
Smart pointers are better than raw pointers. I think I like this patch, but there is also some weirdness that ThreadInfo owns ThreadProfile objects, and ThreadProfile objects can own ThreadInfo objects (though apparently not at the same time?). We should sort that out.
Attachment #8823388 -
Flags: review?(mstange)
![]() |
Assignee | |
Comment 3•6 years ago
|
||
Smart pointers are better than raw pointers, and this makes clients of PlatformData slightly simpler because they don't have to manage destruction themselves: the new UniquePtr-derived type handles all of that for us.
Attachment #8823389 -
Flags: review?(mstange)
Updated•6 years ago
|
Attachment #8823387 -
Flags: review?(mstange) → review+
Comment 4•6 years ago
|
||
Comment on attachment 8823388 [details] [diff] [review] make ThreadInfo manage mProfile with UniquePtr Review of attachment 8823388 [details] [diff] [review]: ----------------------------------------------------------------- I don't know about the the ThreadInfo vs ThreadProfile ownership situation, but this patch seems like it should be safe either way.
Attachment #8823388 -
Flags: review?(mstange) → review+
Comment 5•6 years ago
|
||
Comment on attachment 8823389 [details] [diff] [review] manage Sampler::PlatformData with UniquePtr Review of attachment 8823389 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. It still feels somewhat unnecessary complex - we're abstracting something out that's known at compile time, right? It may also be unused, because I don't see any callers of GetProfiledThread.
Attachment #8823389 -
Flags: review?(mstange) → review+
Updated•6 years ago
|
Blocks: profiler-cleanup
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/41e0e99837f9 part 1 - make ThreadInfo::mName a UniqueFreePtr; r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/07f764d10078 part 2 - make ThreadInfo manage mProfile with UniquePtr; r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a7c28606f1 part 3 - manage Sampler::PlatformData with UniquePtr; r=mstange
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/41e0e99837f9 https://hg.mozilla.org/mozilla-central/rev/07f764d10078 https://hg.mozilla.org/mozilla-central/rev/e1a7c28606f1
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•