Miscellaneous code cleanups in mozjemalloc.cpp

RESOLVED FIXED in Firefox 58

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

2 years ago
No description provided.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8926281 [details]
Bug 1415454 - Remove the unused arena_bin_t* argument to arena_t::AllocRun.

https://reviewboard.mozilla.org/r/197552/#review202808


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

Comment 6

2 years ago
mozreview-review
Comment on attachment 8926281 [details]
Bug 1415454 - Remove the unused arena_bin_t* argument to arena_t::AllocRun.

https://reviewboard.mozilla.org/r/197552/#review202810


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

Comment 7

2 years ago
mozreview-review
Comment on attachment 8926285 [details]
Bug 1415454 - Replace RedBlackTree::Init with a constructor.

https://reviewboard.mozilla.org/r/197556/#review202812


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`
Assignee

Updated

2 years ago
Attachment #8926285 - Attachment is obsolete: true
Attachment #8926285 - Flags: review?(n.nethercote)
Assignee

Comment 8

2 years ago
(In reply to Mike Hommey [:glandium] from comment #4)
> Created attachment 8926285 [details]
> Bug 1415454 - Replace RedBlackTree::Init with a constructor.
> 
> Before 1412722, RedBlackTree actually needed a constructor to fill the
> sentinel, but now, all it contains is a pointer to a root node, and an
> empty tree just has a null root node. So we can use a constexpr
> constructor for the class, which in most cases removes the need for
> manual initialization/construction (being used as member of constructed
> classes, or globals), except in the case of arena_bin_t, which is never
> constructed.
> 
> Review commit: https://reviewboard.mozilla.org/r/197556/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/197556/

FWIW, this made gcc emit a static initializers for gArenas... to set mArenas.mRoot and mPrivateArenas.mRoot to null... except that's too late because other code has already called malloc before the static initializer runs, so it actually resets both trees...

Comment 9

2 years ago
mozreview-review
Comment on attachment 8926281 [details]
Bug 1415454 - Remove the unused arena_bin_t* argument to arena_t::AllocRun.

https://reviewboard.mozilla.org/r/197552/#review203112
Attachment #8926281 - Flags: review?(n.nethercote) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8926282 [details]
Bug 1415454 - Replace log2 lookup table with FloorLog2.

https://reviewboard.mozilla.org/r/197554/#review203116
Attachment #8926282 - Flags: review?(n.nethercote) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8926280 [details]
Bug 1415454 - Inline MallocBinEasy and MallocBinHard.

https://reviewboard.mozilla.org/r/197550/#review203120
Attachment #8926280 - Flags: review?(n.nethercote) → review+

Comment 12

2 years ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/f64defe0d11e
Inline MallocBinEasy and MallocBinHard. r=njn
https://hg.mozilla.org/integration/autoland/rev/b8acdda17818
Remove the unused arena_bin_t* argument to arena_t::AllocRun. r=njn
https://hg.mozilla.org/integration/autoland/rev/bfa36a25eaf6
Replace log2 lookup table with FloorLog2. r=njn

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f64defe0d11e
https://hg.mozilla.org/mozilla-central/rev/b8acdda17818
https://hg.mozilla.org/mozilla-central/rev/bfa36a25eaf6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.