Closed Bug 963840 Opened 6 years ago Closed 6 years ago

Try to move our allocation practices closer into line with best practices


(Core :: JavaScript Engine, defect)

Not set





(Reporter: terrence, Assigned: terrence)


(Blocks 1 open bug)


(Whiteboard: [leave open)


(1 file)

First step: document what allocators are present, how they relate, and what they should be used for at a high level.
Attachment #8365394 - Flags: review?(wmccloskey)
Comment on attachment 8365394 [details] [diff] [review]

Review of attachment 8365394 [details] [diff] [review]:

Stealing review, since it's trivial. Looks good.
Attachment #8365394 - Flags: review?(wmccloskey) → review+
Blocks: 875863
Moving to general GC performance, since this is a general scheduling issue.

The specific issues we need to address here are:

(1) Make sure that only memory that is freeable by a GC is actually accounted as such on the zone or runtime. Right now the distinction between a MallocProvider and an AllocPolicy is really fuzzy. For instance, Vector and HashTable take an rt parameter. You would think then that by default they use a MallocProvider, since JSRuntime is a MallocProvider. But, no. Since internally the type is an AllocPolicy type, the implicit 1-arg AllocPolicy constructor fires, which creates an AllocPolicy that does not account for memory but instead reports errors.

A potential solution here would be to remove MallocProvider from JSRuntime etc and instead put it on Cell. If one wants to allocate memory for a cell, they would need to go through the cell to get it with cell->malloc(rt, size). This changes nothing in practice, but should make everything clearer by design.

(2) Make freeing memory reduce the malloc load in the zone and runtime counters. Right now we don't do this because we sometimes have no way to get the length that is being freed. I don't think we need a 100% solution for this to be useful. Even reducing the malloc trigger pressure for /most/ allocations would be a huge help here.
Blocks: GC.performance
No longer blocks: 875863
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1033442
You need to log in before you can comment on or make changes to this bug.