Closed Bug 1160149 Opened 4 years ago Closed 4 years ago

Frequent SM(cgc) basic/testManyVars.js timeouts

Categories

(Core :: JavaScript: GC, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox40 --- affected

People

(Reporter: RyanVM, Unassigned)

References

Details

Per discussion w/ Jon, I'm going to annotate the test for now. But he wanted to investigate in case there's more at play.

Timeouts started in the vicinity of:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bcbd4118f341
On further investigation, changeset 75d6145145ec makes the runtime of this test go from 36.94 to 63.26 seconds so this is a real issue and bug 1158353 is implicated.

The test does build up a huge rope so it's logical that a string marking change could cause this.  But I can't see any functional different in that patch.  Something to do with inlining perhaps?
Blocks: 1158353
Flags: needinfo?(terrence)
Thanks for tracking that down! I'll try to figure out what changed today.
Flags: needinfo?(terrence)
Okay, I managed to restore the previous performance by changing mark(right)/mark(left) => right->markIfUnmarked()/left->markIfUnmarked(). e.g. The sole difference is the additional assertions (probably mostly CheckTracedThing) that I've added to debug builds.

I'd go so far as to say that this is desirable and we should continue skipping this test in cgc if it means we can have rope-internal correctness assertions everywhere else. Please re-open if you disagree.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.