Closed Bug 533148 Opened 15 years ago Closed 15 years ago

OOM trace-test takes minutes to complete

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: bzbarsky)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

trace-test/tests/basic/testBug507425.js went from taking ~2s to 2 minutes on my Ubuntu laptop with 1GB RAM starting with the cset http://hg.mozilla.org/tracemonkey/rev/c27f496d0d7a from bug 521423. The test grows a string until OOM. I see that the value of JSString::MAX_LENGTH was not changed, but perhaps something else was changed that caused OOMs to happen much later?
Hmm. Looking at the code this should run, I wouldn't have thought it would be affected by those changes. Does which iteration of that loop we throw on change across that changeset? If so, from what to what?
So over here I do see it change from i == 27 to i == 28 on that changeset (Mac laptop, 4GB of RAM). But it completes quite fast, at least in shell.
Oh, and MAX_LENGTH did change: from 0x0FFFFFFF to 0x10000000
Heh, it would appear that the one-greater MAX_LENGTH is the reason; before the test would OOM while still in RAM and now it has to do ~1GB pagefile i/o before OOMing. I'm guessing the same issue caused a random failure on a similar OOM test in bug 523793 comment 24. This may be a problem with OOM tests in the tree that use strings with power-of-2 sizes and now get an extra iteration that is bad for old laptops and crowded VMs. So: (1) testers' fault for using puny hw, (2) tests' faults for walking the line, (3) maybe MAX_LENGTH = (1 << 28) - 1 was better?
This test takes forever for me to run now too.
bz: any issues you can see in going back one to the old MAX_LENGTH? /be
Attached patch Fix (deleted) — Splinter Review
Not at all, apart from this all-hands eating all my time issue. ;)
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
Attachment #416668 - Flags: review?(brendan)
Blocks: 521423
Comment on attachment 416668 [details] [diff] [review] Fix >+ /* Generous but sane length bound; the "-1" is there for backwards-compat. */ >+ static const size_t MAX_LENGTH = ((1 << 28) - 1); Nits: no need to overparenthesize the whole initializer; instead of "backwards-compat" how about "compatibility with OOM tests" or some such (major comment style as needed). /be
Attachment #416668 - Flags: review?(brendan) → review+
Pushed http://hg.mozilla.org/tracemonkey/rev/20e1a6297a97 with the nits. In my defence on the parens, I missed this being a static and not a macro! ;)
Whiteboard: fixed-in-tracemonkey
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: