Bug 50709 - running chromium DRT under heavy concurrency kills Firefox, causes Adium to spin
Summary: running chromium DRT under heavy concurrency kills Firefox, causes Adium to spin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks: 50704
  Show dependency treegraph
 
Reported: 2010-12-08 13:12 PST by Dirk Pranke
Modified: 2012-09-03 01:05 PDT (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2010-12-08 13:12:22 PST
this is an offshoot of bug 50704 - I noticed it while testing that bug, but I suspect it's a different issue.

If you run new-run-webkit-tests with the concurrency patch in bug 48820, then if you run with 6 or more child processes and watch the Activity Monitor, you'll notice that Adium starts to spike CPU load, as does Firefox. In addition Firefox may crash outright. Running with TestShell instead of DRT, or running Mac DRT, does not cause these problems.

Maybe Chromium DRT is contended for some common Mac resource with the other applications, and the problems don't show up unless there's heavy contention?
Comment 1 Tony Chang 2010-12-08 13:48:53 PST
Kent-san works on a mac.  Maybe he can investigate?
Comment 2 Kent Tamura 2010-12-08 17:13:31 PST
ok, I'll try to reproduce and investigate this.
Comment 3 Kent Tamura 2010-12-08 22:00:14 PST
I confirmed Adium's 100% CPU load even with single child, "new-run-webkit-tests --debug --chromium".
As looking at Activity Monitor carefully, Adium have spiked CPU load just for 8 seconds since a DRT process start.  If a DRT process is killed and restarted by a TIMEOUT test, Adium spikes again for 8 seconds.
Comment 4 Kent Tamura 2010-12-09 01:31:10 PST
Well, I found ATSFontActivateFromFileReference() call in webkit/support/platform_support_mac.mm was the culprit.  test_shell also uses it, but test_shell activates only one font though DRT activates 10 fonts (Ahem + WebKitWeightWatcherN00).

I'm not sure if we really need WebKitWeightWatcher fonts.
I tested NRWT --child-process=10 without WebKitWeightWatcher fonts.  It was good though Adium sometimes spiked.

Apple Mac port activates fonts with a different way on SnowLeopard.
(See activateFonts() in DumpRenderTree/mac/DumpRenderTree.mm)
CTFontManagerRegisterFontsForURLs() is available since SnowLeopard.  We can't use it?
Comment 5 Dirk Pranke 2010-12-09 15:57:03 PST
I have almost no knowledge of Mac APIs ... we should probably find someone who knows better, like Nico or one of the Apple guys.
Comment 6 Kent Tamura 2010-12-09 16:38:21 PST
(In reply to comment #4)
> I'm not sure if we really need WebKitWeightWatcher fonts.
> I tested NRWT --child-process=10 without WebKitWeightWatcher fonts.  It was good though Adium sometimes spiked.

WebKitWeightWatcher fonts are needed for LayoutTests/fast/css/font-weight-1.html.
Comment 7 Kent Tamura 2010-12-13 16:58:23 PST
(In reply to comment #4)
> Well, I found ATSFontActivateFromFileReference() call in webkit/support/platform_support_mac.mm was the culprit.  test_shell also uses it, but test_shell activates only one font though DRT activates 10 fonts (Ahem + WebKitWeightWatcherN00).

> Apple Mac port activates fonts with a different way on SnowLeopard.
> (See activateFonts() in DumpRenderTree/mac/DumpRenderTree.mm)
> CTFontManagerRegisterFontsForURLs() is available since SnowLeopard.  We can't use it?

Mark, Oliver,
I'd like to know the detail of <rdar://problem/6698023> mentioned in http://trac.webkit.org/changeset/41827.
We found ATSFontActivateFromFileReference() made heavy CPU load for some seconds on SnowLeopard.
Comment 8 Dirk Pranke 2011-01-11 14:30:04 PST
Did we get anywhere with this? I'm still seeing issues ...
Comment 9 Tony Chang 2011-01-28 16:57:09 PST
Kent suggested that we stop loading the WebKitWeightWatcher fonts and marking the test as failing as a work around for now.
Comment 10 Eric Seidel (no email) 2011-01-30 04:16:14 PST
Wow.  This sounds related font-related crashes adam and my desktop machines were seeing when running the commit-queue -- other applications would crash when running the layout tests repeatedly.  Apple folks should see radar 8630760.
Comment 11 Eric Seidel (no email) 2011-01-30 04:18:06 PST
Dave Kilzer was following the ATS crashes in radar 8630760 and might want to see this bug too.
Comment 12 David Kilzer (:ddkilzer) 2011-01-30 07:54:28 PST
(In reply to comment #7)
> (In reply to comment #4)
> > Well, I found ATSFontActivateFromFileReference() call in webkit/support/platform_support_mac.mm was the culprit.  test_shell also uses it, but test_shell activates only one font though DRT activates 10 fonts (Ahem + WebKitWeightWatcherN00).
> 
> > Apple Mac port activates fonts with a different way on SnowLeopard.
> > (See activateFonts() in DumpRenderTree/mac/DumpRenderTree.mm)
> > CTFontManagerRegisterFontsForURLs() is available since SnowLeopard.  We can't use it?
> 
> Mark, Oliver,
> I'd like to know the detail of <rdar://problem/6698023> mentioned in http://trac.webkit.org/changeset/41827.
> We found ATSFontActivateFromFileReference() made heavy CPU load for some seconds on SnowLeopard.

That radar was fixed in Snow Leopard.

(In reply to comment #10)
> Wow.  This sounds related font-related crashes adam and my desktop machines were seeing when running the commit-queue -- other applications would crash when running the layout tests repeatedly.  Apple folks should see radar 8630760.

Eric, since you filed the radar and had a reproducible test case, you were driving the fix.  After you stopped running your own commit queue, there is no one to provide feedback on potential fixes, so that radar is effectively dead in the water.

(In reply to comment #11)
> Dave Kilzer was following the ATS crashes in radar 8630760 and might want to see this bug too.

Someone on Chromium needs to file a radar about this issue using <https://bugreport.apple.com/>.  They can create a free "online" account through <https://connect.apple.com/>.  I can "import" this bug into radar, but then the person filing it won't be able to get any status updates through DTS (Developer Technical Services), and I'm not the correct person to champion its cause.
Comment 13 Eric Seidel (no email) 2011-01-30 11:16:45 PST
(In reply to comment #12)
> Eric, since you filed the radar and had a reproducible test case, you were driving the fix.  After you stopped running your own commit queue, there is no one to provide feedback on potential fixes, so that radar is effectively dead in the water.

I'm aware. :)  Running a commit-queue node on ones personal machine has many many downsides (including the above-mentioned radar), so I'm not really that sad to be done with that phase of cq management for the moment.

Hopefully radar 8630760 will continue to serve as a focus for information like this (or more crash reports if I need to start running the cq locally again).
Comment 14 Dirk Pranke 2011-01-31 22:52:29 PST
(In reply to comment #9)
> Kent suggested that we stop loading the WebKitWeightWatcher fonts and marking the test as failing as a work around for now.

Yes, that seems to work for me.
Comment 15 Kent Tamura 2011-02-01 18:02:46 PST
We removed the code to register WebKitWeightWatcher fonts.

http://crrev.com/73378
http://trac.webkit.org/changeset/77347
Comment 16 Dirk Pranke 2011-02-04 12:46:32 PST
I'm going to close this as WORKSFORME ... I'll open it again if I see new issues.
Comment 17 Nico Weber 2012-08-26 20:38:54 PDT
It sounds like this was a 10.5 issue. Now that we're on 10.6, should this bug be revisited?
Comment 18 Dirk Pranke 2012-08-28 12:29:57 PDT
I haven't noticed anything on Lion in a while, and I haven't tried this on SL in a long time. Are you seeing any issues that would prompt us to want to look again?

(That said, I haven't tried running recently, either. I can certainly do that).
Comment 19 Nico Weber 2012-08-29 11:21:50 PDT
It looks like the bug was "stuff misbehaves on 10.5 if we do X, so let's disable X". But since we no longer use 10.5 and other ports do X, maybe we should start doing it again? (where X is loading the WeightWatcher fonts)
Comment 20 Dirk Pranke 2012-08-29 11:27:01 PDT
(In reply to comment #19)
> It looks like the bug was "stuff misbehaves on 10.5 if we do X, so let's disable X". But since we no longer use 10.5 and other ports do X, maybe we should start doing it again? (where X is loading the WeightWatcher fonts)

I see. Yes, it might be worth trying again.
Comment 21 Nico Weber 2012-08-29 11:30:02 PDT
(In reply to comment #20)
> (In reply to comment #19)
> > It looks like the bug was "stuff misbehaves on 10.5 if we do X, so let's disable X". But since we no longer use 10.5 and other ports do X, maybe we should start doing it again? (where X is loading the WeightWatcher fonts)
> 
> I see. Yes, it might be worth trying again.

https://chromiumcodereview.appspot.com/10893031/
Comment 22 Nico Weber 2012-08-31 12:47:46 PDT
This landed in chromium, and that'll roll into webkit soon. The failing test should start probably start passing soon -- so I think my work here is done.
Comment 23 Kent Tamura 2012-09-03 01:05:54 PDT
I verified Firefox and Adium didn't spike CPU load.

The test result looks wrong.  It should be a separated issue.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=font-weight-1.html