Bug 50709 – running chromium DRT under heavy concurrency kills Firefox, causes Adium to spin

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dirk Pranke]

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?

[Tony Chang]

Kent-san works on a mac.  Maybe he can investigate?

[Kent Tamura]

ok, I'll try to reproduce and investigate this.

[Kent Tamura]

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.

[Kent Tamura]

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?

[Dirk Pranke]

I have almost no knowledge of Mac APIs ... we should probably find someone who knows better, like Nico or one of the Apple guys.

[Kent Tamura]

(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.

[Kent Tamura]

(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.

[Dirk Pranke]

Did we get anywhere with this? I'm still seeing issues ...

[Tony Chang]

Kent suggested that we stop loading the WebKitWeightWatcher fonts and marking the test as failing as a work around for now.

[Eric Seidel (no email)]

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 Seidel (no email)]

Dave Kilzer was following the ATS crashes in radar 8630760 and might want to see this bug too.

[David Kilzer (:ddkilzer)]

(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.

[Eric Seidel (no email)]

(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).

[Dirk Pranke]

(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.

[Kent Tamura]

We removed the code to register WebKitWeightWatcher fonts.

http://crrev.com/73378
http://trac.webkit.org/changeset/77347

[Dirk Pranke]

I'm going to close this as WORKSFORME ... I'll open it again if I see new issues.

[Nico Weber]

It sounds like this was a 10.5 issue. Now that we're on 10.6, should this bug be revisited?

[Dirk Pranke]

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).

[Nico Weber]

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)

[Dirk Pranke]

(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.

[Nico Weber]

(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/

[Nico Weber]

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.

[Kent Tamura]

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