Closed Bug 1401099 Opened 7 years ago Closed 7 years ago

Some C++ification of arena_t-related functions

Categories

(Core :: Memory Allocator, enhancement)

Product:
Core
Component:
Memory Allocator
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(22 files)

Bug 1401099 - Use Gecko style names for arena_t members.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_purge to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move hard_purge_arena to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_new to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_chunk_init to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_chunk_dealloc to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_run_alloc to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_run_dalloc to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_run_split to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_run_trim_head to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_run_trim_tail to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_bin_malloc_easy to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_bin_malloc_hard to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_bin_nonfull_run_get to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_malloc_small to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_malloc_large to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_malloc to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_palloc to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_dalloc_small to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_dalloc_large to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_ralloc_large_shrink to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1401099 - Move arena_ralloc_large_grow to a method of arena_t.
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
I started doing this because I thought it would help me for the partition allocation API, but it turns out it doesn't, at least not immediately, without other changes that will have to happen later. I figured the changes are still useful because they do a lot of changes that align more parts of mozjemalloc to the gecko coding style. But as they might have an impact on how the compiler optimizes things, I won't take chances and will wait for trunk to turn into 58 before actually landing.
Comment on attachment 8909626 [details] Bug 1401099 - Use Gecko style names for arena_t members. https://reviewboard.mozilla.org/r/181108/#review186398
Attachment #8909626 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909627 [details] Bug 1401099 - Move arena_purge to a method of arena_t. https://reviewboard.mozilla.org/r/181110/#review186400
Attachment #8909627 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909628 [details] Bug 1401099 - Move hard_purge_arena to a method of arena_t. https://reviewboard.mozilla.org/r/181112/#review186402
Attachment #8909628 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909629 [details] Bug 1401099 - Move arena_new to a method of arena_t. https://reviewboard.mozilla.org/r/181114/#review186404
Attachment #8909629 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909630 [details] Bug 1401099 - Move arena_chunk_init to a method of arena_t. https://reviewboard.mozilla.org/r/181116/#review186408 ::: memory/build/mozjemalloc.cpp:2658 (Diff revision 1) > if (chunk->ndirty == 0 && old_ndirty > 0) > arena_chunk_tree_dirty_remove(&arena->mChunksDirty, chunk); > } > > -static void > -arena_chunk_init(arena_t *arena, arena_chunk_t *chunk, bool zeroed) > +void > +arena_t::ChunkInit(arena_chunk_t* aChunk, bool aZeroed) This one feels like it would be better as a arena_chunk_t::Init().
Attachment #8909630 - Flags: review?(n.nethercote) → review-
Comment on attachment 8909632 [details] Bug 1401099 - Move arena_run_alloc to a method of arena_t. https://reviewboard.mozilla.org/r/181120/#review186420 ::: memory/build/mozjemalloc.cpp:3286 (Diff revision 1) > void *ret; > > /* Large allocation. */ > size = PAGE_CEILING(size); > malloc_spin_lock(&arena->mLock); > - ret = (void *)arena_run_alloc(arena, nullptr, size, true, zero); > + ret = (void *)arena->RunAlloc(nullptr, size, true, zero); '(void*)' ::: memory/build/mozjemalloc.cpp:3353 (Diff revision 1) > > MOZ_ASSERT((size & pagesize_mask) == 0); > MOZ_ASSERT((alignment & pagesize_mask) == 0); > > malloc_spin_lock(&arena->mLock); > - ret = (void *)arena_run_alloc(arena, nullptr, alloc_size, true, false); > + ret = (void *)arena->RunAlloc(nullptr, alloc_size, true, false); ditto
Attachment #8909632 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909633 [details] Bug 1401099 - Move arena_run_dalloc to a method of arena_t. https://reviewboard.mozilla.org/r/181122/#review186422 ::: memory/build/mozjemalloc.cpp:2912 (Diff revision 1) > { > - arena_chunk_t *chunk; > + arena_chunk_t* chunk; > - size_t size, run_ind, run_pages; > + size_t size, run_ind, run_pages; > > - chunk = (arena_chunk_t *)CHUNK_ADDR2BASE(run); > - run_ind = (size_t)(((uintptr_t)run - (uintptr_t)chunk) > + chunk = (arena_chunk_t*)CHUNK_ADDR2BASE(aRun); > + run_ind = (size_t)(((uintptr_t)aRun - (uintptr_t)chunk) You could do `uintptr_t(aRun)`-style casts.
Attachment #8909633 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909634 [details] Bug 1401099 - Move arena_run_split to a method of arena_t. https://reviewboard.mozilla.org/r/181124/#review186424 ::: memory/build/mozjemalloc.cpp:788 (Diff revision 1) > public: > arena_run_t* RunAlloc(arena_bin_t* aBin, size_t aSize, bool aLarge, bool aZero); > > void RunDalloc(arena_run_t* aRun, bool aDirty); > > + void RunSplit(arena_run_t* aRun, size_t aSize, bool aLarge, bool aZero); Would SplitRun() be a better name? (Likewise DallocRun()?)
Attachment #8909634 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909635 [details] Bug 1401099 - Move arena_run_trim_head to a method of arena_t. https://reviewboard.mozilla.org/r/181126/#review186774 ::: memory/build/mozjemalloc.cpp:790 (Diff revision 1) > > void RunDalloc(arena_run_t* aRun, bool aDirty); > > void RunSplit(arena_run_t* aRun, size_t aSize, bool aLarge, bool aZero); > > + void RunTrimHead(arena_chunk_t* aChunk, arena_run_t* aRun, size_t aOldSize, size_t aNewSize); TrimRunHead?
Attachment #8909635 - Flags: review?(n.nethercote) → review+
(In reply to Nicholas Nethercote [:njn] from comment #28) > Comment on attachment 8909632 [details] > Bug 1401099 - Move arena_run_alloc to a method of arena_t. > > https://reviewboard.mozilla.org/r/181120/#review186420 > > ::: memory/build/mozjemalloc.cpp:3286 > (Diff revision 1) > > void *ret; > > > > /* Large allocation. */ > > size = PAGE_CEILING(size); > > malloc_spin_lock(&arena->mLock); > > - ret = (void *)arena_run_alloc(arena, nullptr, size, true, zero); > > + ret = (void *)arena->RunAlloc(nullptr, size, true, zero); > > '(void*)' How about removing those (void*) casts?
(In reply to Nicholas Nethercote [:njn] from comment #30) > Would SplitRun() be a better name? (Likewise DallocRun()?) And AllocRun?
> How about removing those (void*) casts? Even better!
Comment on attachment 8909636 [details] Bug 1401099 - Move arena_run_trim_tail to a method of arena_t. https://reviewboard.mozilla.org/r/181128/#review186808 ::: memory/build/mozjemalloc.cpp:792 (Diff revision 1) > > void RunSplit(arena_run_t* aRun, size_t aSize, bool aLarge, bool aZero); > > void RunTrimHead(arena_chunk_t* aChunk, arena_run_t* aRun, size_t aOldSize, size_t aNewSize); > > + void RunTrimTail(arena_chunk_t* aChunk, arena_run_t* aRun, size_t aOldSize, size_t aNewSize, bool dirty); TrimRunTail()?
Attachment #8909636 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909637 [details] Bug 1401099 - Move arena_bin_malloc_easy to a method of arena_t. https://reviewboard.mozilla.org/r/181130/#review186820 ::: memory/build/mozjemalloc.cpp:3116 (Diff revision 1) > return (run); > } > > /* bin->runcur must have space available before this function is called. */ > -static inline void * > -arena_bin_malloc_easy(arena_t *arena, arena_bin_t *bin, arena_run_t *run) > +void* > +arena_t::BinMallocEasy(arena_bin_t* aBin, arena_run_t* aRun) MallocBinEasy?
Attachment #8909637 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909637 [details] Bug 1401099 - Move arena_bin_malloc_easy to a method of arena_t. https://reviewboard.mozilla.org/r/181130/#review186820 Or maybe MallocEasyBin?
Comment on attachment 8909638 [details] Bug 1401099 - Move arena_bin_malloc_hard to a method of arena_t. https://reviewboard.mozilla.org/r/181132/#review186826 ::: memory/build/mozjemalloc.cpp:796 (Diff revision 1) > > void RunTrimTail(arena_chunk_t* aChunk, arena_run_t* aRun, size_t aOldSize, size_t aNewSize, bool dirty); > > inline void* BinMallocEasy(arena_bin_t* aBin, arena_run_t* aRun); > > + void* BinMallocHard(arena_bin_t* aBin); MallocHardBin?
Attachment #8909638 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909639 [details] Bug 1401099 - Move arena_bin_nonfull_run_get to a method of arena_t. https://reviewboard.mozilla.org/r/181134/#review186828 ::: memory/build/mozjemalloc.cpp:3068 (Diff revision 1) > > RunDalloc((arena_run_t*)((uintptr_t)aRun + aNewSize), aDirty); > } > > -static arena_run_t * > -arena_bin_nonfull_run_get(arena_t *arena, arena_bin_t *bin) > +arena_run_t* > +arena_t::BinNonFullRunGet(arena_bin_t* aBin) GetNonFullRunBin?
Attachment #8909639 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909640 [details] Bug 1401099 - Move arena_malloc_small to a method of arena_t. https://reviewboard.mozilla.org/r/181136/#review186830
Attachment #8909640 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909641 [details] Bug 1401099 - Move arena_malloc_large to a method of arena_t. https://reviewboard.mozilla.org/r/181138/#review186832 ::: memory/build/mozjemalloc.cpp:3335 (Diff revision 1) > MOZ_ASSERT(QUANTUM_CEILING(size) <= arena_maxclass); > > if (size <= bin_maxclass) { > return (arena->MallocSmall(size, zero)); > } else > - return (arena_malloc_large(arena, size, zero)); > + return (arena->MallocLarge(size, zero)); Remove parens. (Likewise for the MallocSmall call above, which you could do in the earlier patch.)
Attachment #8909641 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909642 [details] Bug 1401099 - Move arena_malloc to a method of arena_t. https://reviewboard.mozilla.org/r/181140/#review186836 ::: memory/build/mozjemalloc.cpp:3335 (Diff revision 1) > + MOZ_ASSERT(QUANTUM_CEILING(aSize) <= arena_maxclass); > > - MOZ_ASSERT(arena); > - MOZ_DIAGNOSTIC_ASSERT(arena->mMagic == ARENA_MAGIC); > - MOZ_ASSERT(size != 0); > - MOZ_ASSERT(QUANTUM_CEILING(size) <= arena_maxclass); > + if (aSize <= bin_maxclass) { > + return MallocSmall(aSize, aZero); > + } > + return MallocLarge(aSize, aZero); I'd use ?: here, because the two return expressions have equal significance.
Attachment #8909642 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909643 [details] Bug 1401099 - Move arena_palloc to a method of arena_t. https://reviewboard.mozilla.org/r/181142/#review186838
Attachment #8909643 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909644 [details] Bug 1401099 - Move arena_dalloc_small to a method of arena_t. https://reviewboard.mozilla.org/r/181144/#review186840
Attachment #8909644 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909645 [details] Bug 1401099 - Move arena_dalloc_large to a method of arena_t. https://reviewboard.mozilla.org/r/181146/#review186842
Attachment #8909645 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909646 [details] Bug 1401099 - Move arena_ralloc_large_shrink to a method of arena_t. https://reviewboard.mozilla.org/r/181148/#review186844 ::: memory/build/mozjemalloc.cpp:814 (Diff revision 1) > > inline void DallocSmall(arena_chunk_t* aChunk, void* aPtr, arena_chunk_map_t *aMapElm); > > void DallocLarge(arena_chunk_t* aChunk, void* aPtr); > > + void RallocLargeShrink(arena_chunk_t* aChunk, void* aPtr, size_t aSize, size_t aOldSize); RallocShrinkLarge?
Attachment #8909646 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909647 [details] Bug 1401099 - Move arena_ralloc_large_grow to a method of arena_t. https://reviewboard.mozilla.org/r/181150/#review186846 ::: memory/build/mozjemalloc.cpp:814 (Diff revision 1) > > void DallocLarge(arena_chunk_t* aChunk, void* aPtr); > > void RallocLargeShrink(arena_chunk_t* aChunk, void* aPtr, size_t aSize, size_t aOldSize); > > + bool RallocLargeGrow(arena_chunk_t* aChunk, void* aPtr, size_t aSize, size_t aOldSize); RallocGrowLarge? Now I'm not sure about that or RallocShrinkLarge... you choose.
Attachment #8909647 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909631 [details] Bug 1401099 - Move arena_chunk_dealloc to a method of arena_t. https://reviewboard.mozilla.org/r/181118/#review186852 ::: memory/build/mozjemalloc.cpp:779 (Diff revision 1) > > bool Init(); > > void ChunkInit(arena_chunk_t* aChunk, bool aZeroed); > > + void ChunkDealloc(arena_chunk_t* aChunk); DeallocChunk
Attachment #8909631 - Flags: review?(n.nethercote) → review+
Comment on attachment 8909630 [details] Bug 1401099 - Move arena_chunk_init to a method of arena_t. https://reviewboard.mozilla.org/r/181116/#review186854 ::: memory/build/mozjemalloc.cpp:774 (Diff revision 1) > */ > arena_bin_t mBins[1]; /* Dynamically sized. */ > > bool Init(); > > + void ChunkInit(arena_chunk_t* aChunk, bool aZeroed); InitChunk
Attachment #8909630 - Flags: review- → review+
(In reply to Nicholas Nethercote [:njn] from comment #39) > GetNonFullRunBin? I went with GetNonFullBinRun.
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/1252fd56a1c9 Use Gecko style names for arena_t members. r=njn https://hg.mozilla.org/integration/autoland/rev/cab516c56578 Move arena_purge to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/d683f3be2720 Move hard_purge_arena to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/77d191082add Move arena_new to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/d5471a774563 Move arena_chunk_init to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/4dc1af92e208 Move arena_chunk_dealloc to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/297976241b7c Move arena_run_alloc to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/85d693377437 Move arena_run_dalloc to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/164be858d88c Move arena_run_split to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/b73ccc81201f Move arena_run_trim_head to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/c6985d5f90ca Move arena_run_trim_tail to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/44b578a65ada Move arena_bin_malloc_easy to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/05baed504ac5 Move arena_bin_malloc_hard to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/ded028a53452 Move arena_bin_nonfull_run_get to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/9aac426a0bb6 Move arena_malloc_small to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/5e4cc07e9af3 Move arena_malloc_large to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/1f0f2e2553d1 Move arena_malloc to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/bc5302021292 Move arena_palloc to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/c581cd625a23 Move arena_dalloc_small to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/bb5f7d39f4b1 Move arena_dalloc_large to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/877f1fbd034c Move arena_ralloc_large_shrink to a method of arena_t. r=njn https://hg.mozilla.org/integration/autoland/rev/526fae334c8b Move arena_ralloc_large_grow to a method of arena_t. r=njn
Blocks: 1052573
Blocks: 1402174
Depends on: 1406732
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: