Closed Bug 1413096 Opened 3 years ago Closed 3 years ago

Miscellaneous code cleanups in mozjemalloc.cpp

Categories

(Core :: Memory Allocator, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(7 files, 1 obsolete file)

No description provided.
hg error in cmd: hg update --clean -r afc2cbb9370f: abort: unknown revision 'afc2cbb9370f'!
Attachment #8923711 - Attachment is obsolete: true
Attachment #8923711 - Flags: review?(n.nethercote)
Comment on attachment 8923690 [details]
Bug 1413096 - Add missing header guard.

https://reviewboard.mozilla.org/r/194816/#review200270
Attachment #8923690 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923691 [details]
Bug 1413096 - Add "using namespace mozilla" to mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/194818/#review200272

Nice.
Attachment #8923691 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923692 [details]
Bug 1413096 - Use CheckedInt for overflow check in calloc.

https://reviewboard.mozilla.org/r/194820/#review200274
Attachment #8923692 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923693 [details]
Bug 1413096 - Remove pow2_ceil in favor of RoundUpPow2.

https://reviewboard.mozilla.org/r/194822/#review200276
Attachment #8923693 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923694 [details]
Bug 1413096 - Remove unnecessary call to ffs.

https://reviewboard.mozilla.org/r/194824/#review200278
Attachment #8923694 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923695 [details]
Bug 1413096 - Replace ffs with MathAlgorithms functions.

https://reviewboard.mozilla.org/r/194826/#review200280
Attachment #8923695 - Flags: review?(n.nethercote) → review+
Comment on attachment 8923696 [details]
Bug 1413096 - Remove SIZEOF_PTR and SIZEOF_PTR_2POW.

https://reviewboard.mozilla.org/r/194828/#review200282

::: memory/build/rb.h:716
(Diff revision 2)
>     */
>  public:
>    class Iterator
>    {
> -    TreeNode* mPath[3 * ((SIZEOF_PTR << 3) - (SIZEOF_PTR_2POW + 1))];
> +    TreeNode* mPath[3 * ((sizeof(void*) << 3) -
> +                         (mozilla::tl::CeilingLog2<sizeof(void*)>::value + 1))];

If you move Log2/LOG2 into rb.h you can use them here. Alternatively, you could remove them and just using tl::CeilingLog2 everywhere; I don't feel like the check done by Log2 is that important.
Attachment #8923696 - Flags: review?(n.nethercote) → review+
(In reply to Nicholas Nethercote [:njn] from comment #18)
> Comment on attachment 8923696 [details]
> Bug 1413096 - Remove SIZEOF_PTR and SIZEOF_PTR_2POW.
> 
> https://reviewboard.mozilla.org/r/194828/#review200282
> 
> ::: memory/build/rb.h:716
> (Diff revision 2)
> >     */
> >  public:
> >    class Iterator
> >    {
> > -    TreeNode* mPath[3 * ((SIZEOF_PTR << 3) - (SIZEOF_PTR_2POW + 1))];
> > +    TreeNode* mPath[3 * ((sizeof(void*) << 3) -
> > +                         (mozilla::tl::CeilingLog2<sizeof(void*)>::value + 1))];
> 
> If you move Log2/LOG2 into rb.h you can use them here. Alternatively, you
> could remove them and just using tl::CeilingLog2 everywhere; I don't feel
> like the check done by Log2 is that important.

I went with a macro because tl::CeilingLog2<foo>::value is a lot of verbosity. How about moving Log2/LOG2 to a Utils.h header?
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/6543990db9d9
Add missing header guard. r=njn
https://hg.mozilla.org/integration/autoland/rev/96f7edfa1fa2
Add "using namespace mozilla" to mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/09890a442cf8
Use CheckedInt for overflow check in calloc. r=njn
https://hg.mozilla.org/integration/autoland/rev/cce540baf354
Remove pow2_ceil in favor of RoundUpPow2. r=njn
https://hg.mozilla.org/integration/autoland/rev/1011d19677c2
Remove unnecessary call to ffs. r=njn
https://hg.mozilla.org/integration/autoland/rev/81ce645a6e48
Replace ffs with MathAlgorithms functions. r=njn
https://hg.mozilla.org/integration/autoland/rev/42f477976fc5
Remove SIZEOF_PTR and SIZEOF_PTR_2POW. r=njn
You need to log in before you can comment on or make changes to this bug.