Closed
Bug 1412949
Opened 7 years ago
Closed 7 years ago
Some code in SystemMemoryReporter might be dead code
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: marco, Assigned: n.nethercote)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
43.46 KB,
patch
|
erahm
:
review+
jld
:
feedback+
|
Details | Diff | Splinter Review |
I've noticed LinuxUtils.cpp is completely uncovered by tests (https://codecov.io/gh/marco-c/gecko-dev/src/1ebd2eff44617df3b82eea7d2f3ca1b60cc591a0/xpcom/base/LinuxUtils.cpp).
The portions of SystemMemoryReporter.cpp which are supposed to call LinuxUtils.cpp are not covered too (https://codecov.io/gh/marco-c/gecko-dev/src/1ebd2eff44617df3b82eea7d2f3ca1b60cc591a0/xpcom/base/SystemMemoryReporter.cpp).
Should we remove these portions now that Firefox OS is no longer actively developed and Firefox probably doesn't have access to those system directories?
Assignee | ||
Comment 1•7 years ago
|
||
It's not completely dead... that code is behind the memory.system_memory_reporter pref, which is false by default. You can turn it on and it works. But it's probably not useful, so I'll write a patch to remove it.
Assignee | ||
Comment 2•7 years ago
|
||
jld, can you see any reason not to remove this?
Attachment #8923646 -
Flags: review?(erahm)
Attachment #8923646 -
Flags: feedback?(jld)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Comment 3•7 years ago
|
||
Comment on attachment 8923646 [details] [diff] [review]
Remove SystemMemoryReporter
Review of attachment 8923646 [details] [diff] [review]:
-----------------------------------------------------------------
Kind of sad to see these go, but I can't think of a good reason to keep them. Can you update the about:memory docs [1] as well?
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/about:memory#System
Attachment #8923646 -
Flags: review?(erahm) → review+
Comment 4•7 years ago
|
||
Comment on attachment 8923646 [details] [diff] [review]
Remove SystemMemoryReporter
I can't think of a good reason to keep this either.
Attachment #8923646 -
Flags: feedback?(jld) → feedback+
Assignee | ||
Comment 5•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d76af6611465cfccaec40098493eedcd0ba1298b
Bug 1412949 - Remove SystemMemoryReporter. r=erahm.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d76af6611465
Remove SystemMemoryReporter. r=erahm.
Comment 7•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Reporter | ||
Updated•7 years ago
|
Blocks: deadcode-codecoverage
You need to log in
before you can comment on or make changes to this bug.
Description
•