Miscellaneous code cleanups in mozjemalloc

RESOLVED FIXED in Firefox 58

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 1 bug)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(10 attachments)

59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
59 bytes, text/x-review-board-request
njn
: review+
Details
Assignee

Description

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

Updated

2 years ago
Attachment #8924100 - Flags: review?(n.nethercote)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 15

2 years ago
Note, there are some formatting decisions from clang-format that I don't really like, but I guess I can live with the resulting code.

Comment 16

2 years ago
mozreview-review
Comment on attachment 8924094 [details]
Bug 1413475 - Remove ssize_t definition from mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/195314/#review200724
Attachment #8924094 - Flags: review?(n.nethercote) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8924095 [details]
Bug 1413475 - Inline _CRT_SPINCOUNT in mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/195316/#review200728
Attachment #8924095 - Flags: review?(n.nethercote) → review+

Comment 18

2 years ago
mozreview-review
Comment on attachment 8924097 [details]
Bug 1413475 - Move MALLOC_DECOMMIT definition closer to that of MALLOC_DOUBLE_PURGE.

https://reviewboard.mozilla.org/r/195320/#review200732
Attachment #8924097 - Flags: review?(n.nethercote) → review+

Comment 19

2 years ago
mozreview-review
Comment on attachment 8924096 [details]
Bug 1413475 - Reorganize #includes in mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/195318/#review200730

::: memory/build/mozjemalloc.cpp:110
(Diff revision 1)
>  //
>  // *****************************************************************************
>  
>  #include "mozmemory_wrap.h"
>  #include "mozjemalloc.h"
> +#include "mozjemalloc_types.h"

Blank line after this one? (To make the sectioning clearer.)

::: memory/build/mozjemalloc.cpp:124
(Diff revision 1)
> +#endif
> +#ifdef XP_DARWIN
> +#include <libkern/OSAtomic.h>
> +#include <mach/mach_init.h>
> +#include <mach/vm_map.h>
> +#endif

ditto
Attachment #8924096 - Flags: review?(n.nethercote) → review+

Comment 20

2 years ago
mozreview-review
Comment on attachment 8924098 [details]
Bug 1413475 - Remove unused MAP_NOSYNC definition in mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/195322/#review200734
Attachment #8924098 - Flags: review?(n.nethercote) → review+

Comment 21

2 years ago
mozreview-review
Comment on attachment 8924099 [details]
Bug 1413475 - Inline STRERROR_BUF in mozjemalloc.cpp.

https://reviewboard.mozilla.org/r/195324/#review200736

It's only used once because you previously commoned up the uses of strerror_r! Nice.
Attachment #8924099 - Flags: review?(n.nethercote) → review+

Comment 22

2 years ago
mozreview-review
Comment on attachment 8924100 [details]
Bug 1413475 - Replace SIZEOF_INT_2POW with LOG2(sizeof(int)).

https://reviewboard.mozilla.org/r/195326/#review200738

This is fine, but `LOG2(sizeof(int)) + 3` is so common I wonder if it's worth putting it in a constant instead.
Attachment #8924100 - Flags: review?(n.nethercote) → review+

Comment 23

2 years ago
mozreview-review
Comment on attachment 8924101 [details]
Bug 1413475 - Normalize license boilerplates in memory/build/.

https://reviewboard.mozilla.org/r/195328/#review200740
Attachment #8924101 - Flags: review?(n.nethercote) → review+
Assignee

Comment 24

2 years ago
(In reply to Nicholas Nethercote [:njn] from comment #22)
> Comment on attachment 8924100 [details]
> Bug 1413475 - Replace SIZEOF_INT_2POW with LOG2(sizeof(int)).
> 
> https://reviewboard.mozilla.org/r/195326/#review200738
> 
> This is fine, but `LOG2(sizeof(int)) + 3` is so common I wonder if it's
> worth putting it in a constant instead.

That's the kind of code that will go away when arena_run_t gains some methods, so I wouldn't worry too much about this for now.

Comment 25

2 years ago
mozreview-review
Comment on attachment 8924102 [details]
Bug 1413475 - Change comments to use C++ style // instead of /* */ in memory/build/.

https://reviewboard.mozilla.org/r/195330/#review200746
Attachment #8924102 - Flags: review?(n.nethercote) → review+

Comment 26

2 years ago
mozreview-review
Comment on attachment 8924103 [details]
Bug 1413475 - Run clang-format on all files in memory/build/.

https://reviewboard.mozilla.org/r/195332/#review200748

The lack of indenting for preprocessor directives is a shame. Then again, the Mozilla codebase is horribly inconsistent about how to do that...
Attachment #8924103 - Flags: review?(n.nethercote) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 35

2 years ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/9d00b2fff848
Remove ssize_t definition from mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/b16212b3d834
Inline _CRT_SPINCOUNT in mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/5d39a3d4c19b
Reorganize #includes in mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/567d3623e78f
Move MALLOC_DECOMMIT definition closer to that of MALLOC_DOUBLE_PURGE. r=njn
https://hg.mozilla.org/integration/autoland/rev/dd8f08e3a25e
Remove unused MAP_NOSYNC definition in mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/622a6b764ea7
Inline STRERROR_BUF in mozjemalloc.cpp. r=njn
https://hg.mozilla.org/integration/autoland/rev/d5fa1c46befc
Replace SIZEOF_INT_2POW with LOG2(sizeof(int)). r=njn
https://hg.mozilla.org/integration/autoland/rev/1823bf65f460
Normalize license boilerplates in memory/build/. r=njn
https://hg.mozilla.org/integration/autoland/rev/bdf6c8f64bfa
Change comments to use C++ style // instead of /* */ in memory/build/. r=njn
https://hg.mozilla.org/integration/autoland/rev/8f7809346a83
Run clang-format on all files in memory/build/. r=njn
You need to log in before you can comment on or make changes to this bug.