If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

NJ merge: combine the different assembler-reset methods

VERIFIED FIXED

Status

Tamarin
Baseline JIT (CodegenLIR)
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: graydon, Assigned: Rick Reitmaier)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Created attachment 403609 [details] [diff] [review]
import TM changes

This patch pulls in some reset-simplification and asserts from TM's assembler. Applies to TR after ... several of the existing changes (best to land bug 519535 and its dependencies first). It doesn't logically depend on them, I don't think, but might not apply without.
(Assignee)

Comment 1

8 years ago
Created attachment 405110 [details] [diff] [review]
assm reset changes 

Pure speculation, but might it be worthwhile to also call reset() in endAssembly() otherwise we'll have some variables laying about that are non-null, potentially gumming up the gc.
Attachment #403609 - Attachment is obsolete: true
Attachment #405110 - Flags: superreview?(edwsmith)

Updated

8 years ago
Attachment #405110 - Flags: superreview?(edwsmith) → superreview+

Comment 2

8 years ago
Comment on attachment 405110 [details] [diff] [review]
assm reset changes 

Assembler is short-lived and stack allocated in Tamarin, so I don't see any reason to do any more cleanup work than necessary unless we have a legitimate GC bug or performance problem.
(Assignee)

Updated

8 years ago
Blocks: 519527
(Assignee)

Comment 3

8 years ago
pushed http://hg.mozilla.org/tamarin-redux/rev/235ba0da57e9
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 4

8 years ago
Resolved fixed engineering / work item that has been pushed.  Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.