Closed Bug 480621 Opened 16 years ago Closed 14 years ago

Windows 3d-morph regression

Categories

(Core :: JavaScript Engine, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: sayrer, Assigned: jorendorff)

Details

Attachments

(1 file)

data from qm-pxp-trunk-04 Mon Feb 23 20:57:00 2009 62ms Tue Feb 24 08:07:00 2009 89ms http://hg.mozilla.org/tracemonkey/pushloghtml?startdate=2009-02-23+20%3A00%3A00&enddate=2009-02-24+09%3A00%3A00
Flags: blocking1.9.1+
Priority: -- → P2
I tested this using MSVC7.1 (it's what I've got) and got results that are definitely not as damning as sayrer's: 25394:4d2d2fca8c0d : 250 234 234 234 234 234 250 25398:42acafcdae02 : 250 235 250 235 234 250 250 Inconclusive, I think. Something different must be happening in MSVC8. If anyone else is willing to take this, I'd be grateful. If not, I need to get a Windows dev machine.
I was going to try this out today. I have MSVC8 installed.
Are we even running the same versions and OS and everything? My results are so different I can hardly believe I'm running the right test. For me: 25394:4d2d2fca8c0d: 25ms 25398:42acafcdae02: 24ms This is MSVC8/Vista/Core 2 Duo E7300 2.67 GHz.
This is still showing a regression.
I still can't duplicate the regression. I tried 25394:4d2d2fca8c0d vs tip and got about 42 ms in both cases.
Vista, core 2 duo, vc8 toolchain here, likewise can't reproduce. Even cranking the iteration count way up to get a more stable multi-second measure, the range in question shows identical times at both ends, and tip is a solid 10-15% faster than either.
sayrer, do you still see a regression here?
still there. it's not me seeing it, it's the tinderbox.
OK. So: - What is the tinderbox actually running? Is this Tsspider, or what? - What does the line on the graph mean? - What kind of machine/OS/etc is this tinderbox?
I have finally managed to duplicate this. It seems to have occurred on this revision: changeset: 25488:42acafcdae02 user: Jason Orendorff <jorendorff@mozilla.com> date: Mon Feb 23 17:31:02 2009 -0600 summary: Bug 477279 - Tune dense array growth. r=brendan. It happens only with --enable-jemalloc. Without jemalloc, 3d-morph perf is the same on this revision and the previous.
graydon mentioned that for certain large memory sizes jemalloc hits mmap and munmap for every allocation.
In 3d-morph, there seems to be one array that gets created and resized. The capacity goes up to 65535 in this test. Given that there are only 14 calls to ResizeSlots made from EnsureCapacity (in the new version), it's hard to see how they could take a full 14 ms longer than previously. I was wondering if having the array bigger than needed could make GC take longer, but that doesn't make a lot of sense given that this happens only for jemalloc builds, and AFAICT, only in the browser.
Comment on attachment 377049 [details] [diff] [review] untested: limit dense array trace to largest possibly-set value Looks good to me, hard to believe capacity outraces length so much that it would matter very often, but definitely worth doing regardless.
Attachment #377049 - Flags: review+
Assignee: general → jorendorff
Even --enable-threadsafe --enable-jemalloc I can't reproduce this in the shell on Linux. I verified with objdump that js is actually linked with libjemalloc.a. 25394:4d2d2fca8c0d 123.8ms +/- 4.7% 25398:42acafcdae02 120.8ms +/- 2.8%
Sorry, wrong numbers. 25394:4d2d2fca8c0d 50.9ms +/- 5.2% 25398:42acafcdae02 51.4ms +/- 6.2%
(In reply to comment #16) > Even --enable-threadsafe --enable-jemalloc I can't reproduce this in the shell > on Linux. I verified with objdump that js is actually linked with > libjemalloc.a. I have only been able to duplicate this with all of these selections: - Windows - --enable-jemalloc - browser Talos tsspider test Windows may not be required; I didn't test the same way on other platforms. But I found that on Windows I could not duplicate without using jemalloc. And no other test harness but browser Talos could duplicate it.
Guys, how are we doing on this? Do we have enough information to tell where exactly the problem exists? Also, who is actively working on this, Jason or dmandelin? Appreciate any update.
I'm working on it. Hopefully jorendorff is as well. So far I have determined that the gc patch doesn't help, that the first call site to EnsureCapacity (around line 457 or so) is the one to blame, and that it doesn't *seem* any extra time is being spent in ResizeSlots in the regressed version.
Flags: wanted1.9.1+
Flags: blocking1.9.2+
Flags: blocking1.9.1+
Flags: blocking1.9.2+ → blocking1.9.2-
These bugs are all part of a search I made for js bugs that are getting lost in transit: http://tinyurl.com/jsDeadEndBugs They all have a review+'ed, non-obsoleted patch and are not marked fixed-in-tracemonkey or checkin-needed but have not seen any activity in 300 days. Some of these got lost simply because the assignee/patch provider never requested a checkin, or just because they were forgotten about.
We've long surpassed that perf.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: