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

TM: Better coordination of nested tree recording

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: gal, Assigned: gal)

Tracking

({verified1.9.1})

unspecified
x86
Mac OS X
verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
The attached patch improves the coordination of nested tree recording. When we abort an outer recorder because we wait for a) an inner tree to be compiled b) an inner tree to finish growing a branch or c) an inner tree to grow around its inner tree, remember in that recording the loop header of the loop we just aborted, and in case that compilation succeeds, permit all peer fragments attached to that particular loop header to compile at least one more time. If the location was permanently blacklisted, undo the blacklisting. With this patch we can drop BL_ATTEMPTS to 2 (from 6) and still get proper tree construction for our favorite trouble makers fasta and fannkuch. This wins about 10ms on SS and should overall reduce the recording overhead in cases where we abort recording repeatedly since we blacklist faster now.
(Assignee)

Comment 1

9 years ago
Created attachment 365815 [details] [diff] [review]
patch
Assignee: general → gal
(Assignee)

Updated

9 years ago
Attachment #365815 - Flags: review?(dmandelin)
(Assignee)

Comment 2

9 years ago
Note also this subtle change:

int32_t& hits = c->hits();
    if (outer || hits++ >= HOTEXIT && hits <= HOTEXIT+MAXEXIT) {
        /* start tracing secondary trace from this point */
        c->lirbuf = f->lirbuf;

dmandelin identified this problem. We used to wait for HOTEXIT when an already recorded inner tree tried to grow a branch while we were recording the outer tree. This caused repeated recording aborts. If we already abort recording because of such an inner tree growth, we of course want to immediately record no matter what hotexit says.
Comment on attachment 365815 [details] [diff] [review]
patch

Looks good to me. I'll do my own SS test on it a bit later (since my machine tends to be different) but I think it's good to land based on what we know.
Attachment #365815 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 4

9 years ago
http://hg.mozilla.org/tracemonkey/rev/ec90dd58f1da
Whiteboard: fixed-in-tracemonkey
Depends on: 482271

Comment 5

9 years ago
http://hg.mozilla.org/mozilla-central/rev/ec90dd58f1da
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 6

9 years ago
http://hg.mozilla.org/tracemonkey/rev/3fb1b5d6dc60
/cvsroot/mozilla/js/tests/js1_8_1/trace/trace-test.js,v  <--  trace-test.js
new revision: 1.13; previous revision: 1.12
Flags: in-testsuite+

Comment 7

9 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8644a9f87bdc
Keywords: fixed1.9.1

Comment 8

9 years ago
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Depends on: 489836
You need to log in before you can comment on or make changes to this bug.