Closed Bug 765845 Opened 9 years ago Closed 9 years ago
Hang in Draw
Target D2D::Fill Glyphs
Whenever I use my laptop on battery Firefox seems to slow to a crawl. I am becoming more and more convinced that the slowdown I see is due to our 3d acceleration, probably because NVIDIA switches to a very slow low-power mode. Recently I started collecting minidumps whenever Firefox enters a hang state (where the window gets greyed out and the title bar says "Not Responding"). I collected about ten different dumps and each one had the following stack: Main thread: DWrite.dll!operator!=() + 0x19 bytes DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::IsEqualKeyImpl() + 0x3a bytes DWrite.dll!CacheReader::LookupElement() + 0xe6 bytes DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::FillFromReadOnlyCache() + 0x76 bytes DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::ServerLookup() + 0xa8 bytes DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::GetGlyphData() + 0x57 bytes DWrite.dll!GetGlyphBitmaps() + 0x93 bytes DWrite.dll!GlyphRunAnalysis::GlyphRunAnalysis() + 0x2fc bytes DWrite.dll!DWriteFactory::CreateGlyphRunAnalysis() + 0xfb bytes d2d1.dll!GlyphRunRealizer::Init() + 0xf8 bytes d2d1.dll!CHwSurfaceRenderTarget::DrawGlyphRunInternal() + 0xbc bytes d2d1.dll!CCommand_DrawGlyphRun::Execute() + 0x27 bytes d2d1.dll!CHwSurfaceRenderTarget::ProcessBatch() + 0x4c bytes d2d1.dll!CBatchSerializer::FlushInternal() + 0x2e bytes d2d1.dll!CBatchSerializer::FlushInternalAndTryIncreaseStorage() + 0x36 bytes d2d1.dll!CBatchSerializer::StartCommand<CCommand_SetTextRenderingParams>() + 0xc bytes d2d1.dll!DrawingContext::SetTextRenderingParams() + 0x34 bytes d2d1.dll!D2DRenderTargetBase<ID2D1HwndRenderTarget>::SetTextRenderingParams() + 0x4e bytes > gkmedias.dll!mozilla::gfx::DrawTargetD2D::FillGlyphs(mozilla::gfx::ScaledFont * aFont, const mozilla::gfx::GlyphBuffer & aBuffer, const mozilla::gfx::Pattern & aPattern, const mozilla::gfx::DrawOptions & aOptions, const mozilla::gfx::GlyphRenderingOptions * aRenderOptions) Line 886 xul.dll!gfxContext::AzureState::AzureState(const gfxContext::AzureState & __that) + 0x80 bytes 00388a04() 000000aa() 4a7faa24() Is there anything that can be done about this? Here are my graphics stats: Adapter Description Intel(R) HD Graphics Family Vendor ID 0x8086 Device ID 0x0126 Adapter RAM Unknown Adapter Drivers igdumd64 igd10umd64 igd10umd64 igdumdx32 igd10umd32 igd10umd32 Driver Version 18.104.22.1688 Driver Date 9-26-2011 Adapter Description (GPU #2) NVIDIA Quadro 2000M Vendor ID (GPU #2) 0x10de Device ID (GPU #2) 0x0dda Adapter RAM (GPU #2) 2048 Adapter Drivers (GPU #2) nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um Driver Version (GPU #2) 22.214.171.12493 Driver Date (GPU #2) 8-12-2011 Direct2D Enabled true DirectWrite Enabled true (6.1.7601.17789) ClearType Parameters ClearType parameters not found WebGL Renderer Blocked for your graphics card because of unresolved driver issues. GPU Accelerated Windows1/1 Direct3D 10 AzureBackend direct2d
Hrm, we will be calling this a little more with Azure content than without. Has this problem started recently? The fix should be easy. But I should fix this on aurora as well.
This makes us call TextRenderingParams as often as we do with Cairo. I never really knew this could cause a flush of the command stream so could be expensive.
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #634260 - Flags: review?(jmuizelaar)
(In reply to Bas Schouten (:bas) from comment #1) > Has this problem started recently? Not sure about this particular stack, but FF has been really slow on battery power for a while now. I try to be plugged in whenever I can because of it so I just don't know.
It's not clear that this is a recent regression, so no need to track for release. That being said, we would consider an uplift to Aurora 15 once fixed.
If this patch fixes the bug, then it's a regression from bug 715768, which landed for Firefox 15. Ben, can you do a build with this patch in it?
Wait, if this reduces the number of times we call this function that clearly causes an expensive blocking operation why wouldn't we take it? I can do a build sometime soon, I guess.
Oh, we'd take it, but I'd like to know if it isn't fixed :)
Oops, bugzilla changed the flags...
So... It seems that this changes rendering slightly. It's hard to see it, almost. I'll attach some screenshots.
Comment on attachment 634260 [details] [diff] [review] Reduce calling frequency of SetTextRenderingParams Review of attachment 634260 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetD2D.h @@ +199,5 @@ > mutable RefPtr<ID2D1RenderTarget> mRT; > > + // We store this to prevent excessive SetTextRenderingParams calls. > + RefPtr<IDWriteRenderingParams> mTextRenderingParams; > + This should say why we want to avoid excessive SetTextRenderingParams calls, but maybe that is better off at the SetTextRenderingParams call site.
Attachment #634260 - Flags: review?(jmuizelaar) → review+
Though it is also important to know why the rendering changed.
It looks to me like there are two differences in the font rendering: different hinting behavior (maybe the difference between Natural and GDI-like rendering, for example?), and different gamma correction being used for the colored subpixel-AA pixels.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #14) > Though it is also important to know why the rendering changed. I guess I could see this happening if it returned the TempRT for drawing somewhere... I'll add something for that to the patch.
The verdict is in, though: this patch fixes the hang I was seeing. Now I see some slowdown on battery but all my profiles point to the layout engine.
This is a regression then, let's fix it. I'll be uploading a new patch in a little bit.
The previous patch was very broken (note how it sets mParams before actually checking if it's equal to params). This patch fixes all correctness issues.
Attachment #636098 - Flags: review?(jmuizelaar) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Comment on attachment 636098 [details] [diff] [review] Reduce calling frequency of SetTextRenderingParams v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): 715768 User impact if declined: Severe performance slowdown in some cases Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): Very low risk String or UUID changes made by this patch: None
Attachment #636098 - Flags: approval-mozilla-aurora?
Comment on attachment 636098 [details] [diff] [review] Reduce calling frequency of SetTextRenderingParams v2 [Triage Comment] New regression in FF15, with a low risk fix. Approved for Aurora 15.
Attachment #636098 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
[Triage Comment] This missed landing in when 15 was on Aurora so now needs to land to beta. Please update the status flags once this has landed.
Comment on attachment 636098 [details] [diff] [review] Reduce calling frequency of SetTextRenderingParams v2 [Triage Comment] bumping up the approval nomination accordingly.
Attachment #636098 - Flags: approval-mozilla-aurora+ → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.