Closed Bug 516620 Opened 15 years ago Closed 15 years ago

NJ merge: Kill off residue of MMgc

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: graydon, Assigned: graydon)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files, 1 obsolete file)

Update the patch
41.38 KB, patch
gal
: review+
Details | Diff | Splinter Review
match user-defined operator new with delete
1.65 KB, patch
graydon
: review-
Details | Diff | Splinter Review
Attached patch Chop, chop, chop. (obsolete) — — Splinter Review 
Oh, such magnificent deletion.
Attachment #400632 - Flags: review?(gal)
Comment on attachment 400632 [details] [diff] [review]
Chop, chop, chop.

This makes the tempAlloc even more important. Working on it.
Attachment #400632 - Flags: review?(gal) → review+
http://hg.mozilla.org/tracemonkey/rev/35777195800a
Whiteboard: fixed-in-tracemonkey
Backed out, perf regression. Will hunt tomorrow.
Whiteboard: fixed-in-tracemonkey
Attached patch Update the patch — — Splinter Review 
This update corrects the leak that was (as far as I can tell) the source of the regression. Tracker pages weren't being allocated in the Allocator.
Attachment #400632 - Attachment is obsolete: true
Attachment #400927 - Flags: review?(gal)
Attachment #400927 - Flags: review?(gal) → review+
Comment on attachment 400927 [details] [diff] [review]
Update the patch

This is moving a ton of data into the allocator. We really need that tempAlloc. Feel free to steal that bug from me. I was thinking codeAlloc/dataAlloc and then tempAlloc which is a member of TraceRecorder and NativeRegExpCompiler. For that those would have to stay new/delete allocated. Or we create the temp allocator dynamically and throw it away in the old destructor/now thrashTrees().
http://hg.mozilla.org/tracemonkey/rev/c256a60dd22c

Landed a more-conservative subset of the patch that only removes MMgc-ness, doesn't move anything into allocators. Uses operator new and calloc explicitly.

Can play with allocators later.
Whiteboard: fixed-in-tracemonkey
Depends on: 517299
valgrind complains about a mismatched calloc / operator delete.  I think technically it's justified, since operator delete is not required to just call std::free.  The patch adds an explicit delete that calls free.
Attachment #401904 - Flags: review?(graydon)
Luke: Jesse already filed, I'm fixing pending r? from gal, see bug 517299.
Attachment #401904 - Flags: review?(graydon) → review-
Comment on attachment 401904 [details] [diff] [review]
match user-defined operator new with delete

Landed elsewhere (bug 517299), thanks though!
Blocks: 503556
http://hg.mozilla.org/mozilla-central/rev/c256a60dd22c
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: