Avoid potential for wasted space in array_new due to jemalloc rounding up

RESOLVED DUPLICATE of bug 787246

Status

()

RESOLVED DUPLICATE of bug 787246
7 years ago
6 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

7 years ago
This is spun off from bug 685429.  array_new (all three variants) add a count to the start.  If you asked for a power-of-two size, this bumps you up to the next size class, which is often the next power-of-two.  Bug 685429 spot-fixed the

pbiggar said:

> array_new is a trap for the unwary. Can we rename it to
> array_new_with_alignment_trap? Or make a version which doesn't call
> destructors and doesn't need the size stored? Perhaps
> array_new<WeDontCallDestructorsForYou> vs array_new<ThisAddsAnIntToYourSize>.

Luke suggested:

> A last idea: what if we required the caller to pass the size to delete?
> I suspect we usually have it on hand.  In debug, we could store the size to
> assert that the passed size was correct.

There was also the suggestion of using malloc_usable_size, but it's not guaranteed to be available on all platforms, and is difficult to use in the JS engine anyway (see bug 676732 for details).

There aren't many existing uses of array_new, they're mostly in CTypes.cpp, and I don't think any of them involve types with destructors.  Assuming that's correct, the easiest solution would be to rename array_delete as array_delete_no_destructors_run.  Worse is better! :)

Comment 1

7 years ago
If we go the no-destructors route (which I also like) then perhaps s/array_new/malloc_array/ (and not call constructors; effectively malloc_array<T>(n) = malloc(n * sizeof(T))) and s/array_delete/free_/.  That way there is no ambiguity nor verbosity: you get no constructors/destructors; if you want that, use js::Vector or something.
(Assignee)

Comment 2

7 years ago
(In reply to Luke Wagner [:luke] from comment #1)
> That way there is no ambiguity nor verbosity: you get no
> constructors/destructors; if you want that, use js::Vector or something.

Good idea, I'll do that.
(Assignee)

Comment 3

6 years ago
Bug 787246 will remove array_new.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 787246
You need to log in before you can comment on or make changes to this bug.