Finish the allocator refactoring

RESOLVED FIXED in Firefox 49

Status

()

Core
JavaScript: GC
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8751851 [details] [diff] [review]
00_finish_moving_to_Allocator-v0.diff

This moves the rest of the allocation code into Allocator.cpp and makes the following trivial changes:

* return TenuredCell from refillFreeList
* use refillFreeListInGC to implement tenuring allocations

The rest is just code motion.

With this, I believe that all of the core allocation logic is pretty well isolated in Allocator.cpp.
Attachment #8751851 - Flags: review?(jcoppeard)
(Assignee)

Updated

2 years ago
Blocks: 988356
Comment on attachment 8751851 [details] [diff] [review]
00_finish_moving_to_Allocator-v0.diff

Review of attachment 8751851 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

::: js/src/gc/Allocator.cpp
@@ +358,5 @@
> +     */
> +
> +    zone->arenas.checkEmptyFreeList(thingKind);
> +    mozilla::DebugOnly<JSRuntime*> rt = zone->runtimeFromMainThread();
> +    MOZ_ASSERT(rt->isHeapMajorCollecting() || rt->isHeapMinorCollecting());

You can just assert rt->isHeapCollecting() here.

@@ +434,5 @@
>      }
>  }
> +
> +
> +// ///////////  Chunk -> Arena Allocator  //////////////////////////////////////

Total nit, but we should probably standardise this kind of 'banner comment'.  I know I use something different for this.

@@ +531,5 @@
> +uint32_t
> +Chunk::findDecommittedArenaOffset()
> +{
> +    /* Note: lastFreeArenaOffset can be past the end of the list. */
> +    for (unsigned i = info.lastDecommittedArenaOffset; i < ArenasPerChunk; i++)

Pre-existing, but these for loops need braces.
Attachment #8751851 - Flags: review?(jcoppeard) → review+
(Assignee)

Comment 2

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e5b2a68ee2b
(Assignee)

Comment 3

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9bf3d65f53d
(Assignee)

Comment 4

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04ecef934d11
(Assignee)

Comment 5

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/00d30d448554e802975a76a9572911b902a1cb7f
Bug 1272449 - Finish refactoring allocator code; r=jonco
(Assignee)

Comment 6

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/418fd092a81ab94faa0f4acdb4c0da36f1e0e7f2
Bug 1272449 - Followup to fix unified bustage; r=meow

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/00d30d448554
https://hg.mozilla.org/mozilla-central/rev/418fd092a81a
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.