499980 - BitSet doesn't support fixed allocation

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Edwin Smith]

Attachment: allow GC* == NULL and use fixed allocation instead

avmplus::BitSet requires GC memory if the bitset grows.  We need it in contexts where we want to use fixed memory instead of gc memory.  Easy enough to modify it to accept NULL GC* and use global new/delete instead, like SortedMap<> and List<> already do.

Comment 1

[Tommy Reilly]

I can think of no reason to use GC memory for bits, in fact I suspect it was written that way by someone not on our team who was just copying code from avmplus::List and didn't really understand that they didn't have to use GC memory.

Comment 2

[Edwin Smith]

true, this whole class can be fixed-mem.  sweet.

Comment 3

[Steven Johnson]

yeah -- I can review+ this if you like but IMHO better to nuke the GC options and always call the FixedMalloc code directly.

Comment 4

[Edwin Smith]

Attachment: remove GC stuff from BitSet entirely

Debugger can make BitSet be a class field since its already a GCFinalizedObject, and other uses of BitSet on the stack are contexts that wont be unwound by an Exception.  hence, BitSet can be plain C++ and use plain new/delete for its field.

Comment 5

[Steven Johnson]

Comment on attachment 384681 [details] [diff] [review]
remove GC stuff from BitSet entirely

any risk that the dtors can be skipped if longjmp is called?

Comment 6

[Edwin Smith]

actually yes, this patch depends on CodegenLIR properly cleaning itself up after an exception, which is done in bug 499672.

Comment 7

[Edwin Smith]

pushed 
http://hg.mozilla.org/tamarin-redux/rev/41e575e6eb2671cee673feeaf302ca2131919ee9

Comment 8

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.