Project: skia Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 4 users
Status: Accepted
Owner:
Area: ----
Priority: Low
Type: Enhancement



Sign in to add a comment
Bottleneck in SkGlyphCache::VisitCache
Project Member Reported by tomhud...@chromium.org, Jun 4 2013 Back to list
Loading Wikipedia's List_of_Unicode_characters into Chrome for Android, we see SkGlyphCache::VisitCache() being hot. I don't trust perf annotate for this function (which is suggesting that one opcode is responsible for 50% of the time), but there might be scope for optimization:

     4.63%  ChildProcessMai  [kernel]                         [k] 0xc00094b4
     4.29%  ChildProcessMai  libchromeview.so                 [.] SkGlyphCache::VisitCache(SkTypeface*, SkDescriptor const*, bool (*)(SkGlyphCache const*, void*), void*)
     2.63%  ChildProcessMai  libchromeview.so                 [.] WTF::Vector<float, 0u>::operator[](unsigned int)
     2.37%  ChildProcessMai  libchromeview.so                 [.] WebCore::RenderTable::colToEffCol(unsigned int) const
     2.17%  ChildProcessMai  libc.so                          [.] dlmalloc
     2.00%  ChildProcessMai  libc.so                          [.] 0xe038  
     1.81%  ChildProcessMai  libchromeview.so                 [.] 0x1e1e98
     1.68%  ChildProcessMai  libchromeview.so                 [.] bool WTF::Vector<WebCore::QualifiedName, 0u>::contains<WebCore::QualifiedName>(WebCore::QualifiedName const&) const
     1.60%  ChildProcessMai  libc.so                          [.] dlfree
     1.41%  ChildProcessMai  libchromeview.so                 [.] WebCore::TextAutosizer::contentHeightIsConstrained(WebCore::RenderBlock const*)
     1.40%  ChildProcessMai  libchromeview.so                 [.] v8::internal::IC::megamorphic_stub_strict()
     1.13%  ChildProcessMai  libchromeview.so                 [.] WebCore::CollapsedBorderValue::CollapsedBorderValue(WebCore::BorderValue const&, WebCore::Color const&, WebCore::EBorderPreced

 
Project Member Comment 1 by caryclark@google.com, Sep 2 2015
Owner: herb@google.com
Status: Accepted
Herb, not sure if this old bug is related to anything you've looked at so far..
Project Member Comment 2 by herbde...@gmail.com, Sep 2 2015
These finding are consistent with what I have found. This cl: https://codereview.chromium.org/1264103003/ should fix the hot spotting problems.
Project Member Comment 4 by bugdroid1@chromium.org, Sep 4 2015
The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/ef2df09997f5afa836bbffdab1ff732d0c766a93

commit ef2df09997f5afa836bbffdab1ff732d0c766a93
Author: herb <herb@google.com>
Date: Fri Sep 04 21:19:45 2015

Revert of Parallel cache - preliminary  (patchset #22 id:420001 of https://codereview.chromium.org/1264103003/ )

Reason for revert:
Seems to freeze android devices.

Original issue's description:
> Parallel cache.
>
> TBR=reed@google.com
>
> BUG=skia:1330
>
> Committed: https://skia.googlesource.com/skia/+/6f2a486040cb25465990196c229feb47e668e87f

TBR=reed@google.com,mtklein@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:1330

Review URL: https://codereview.chromium.org/1327703003

[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/include/core/SkAtomics.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/include/ports/SkAtomics_atomic.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/include/ports/SkAtomics_std.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/include/ports/SkAtomics_sync.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/src/core/SkDraw.cpp
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/src/core/SkGlyph.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/src/core/SkGlyphCache.cpp
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/src/core/SkGlyphCache.h
[modify] http://crrev.com/ef2df09997f5afa836bbffdab1ff732d0c766a93/src/core/SkGlyphCache_Globals.h

Project Member Comment 6 by bugdroid1@chromium.org, Sep 11 2015
The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/951d8543271ff541a45576bea207b33748802d2e

commit 951d8543271ff541a45576bea207b33748802d2e
Author: jyasskin <jyasskin@chromium.org>
Date: Fri Sep 11 01:11:29 2015

Revert of Parallel cache - preliminary  (patchset #23 id:440001 of https://codereview.chromium.org/1264103003/ )
Also reverts https://codereview.chromium.org/1333003002/ which was layered on top.

Reason for revert:
Appears to leak GDI handles: http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%282%29/builds/8247

~~Dr.M~~ Error #1: HANDLE LEAK: GDI handle 0x03050a84 and 3 similar handle(s) were opened but not closed:
~~Dr.M~~ # 0 system call NtGdiCreateDIBSection
~~Dr.M~~ # 1 GDI32.dll!CreateDIBSection                                                +0xdc     (0x768ead23 <GDI32.dll+0x1ad23>)
~~Dr.M~~ # 2 skia.dll!HDCOffscreen::draw                                                [third_party\skia\src\ports\skfonthost_win.cpp:499]
~~Dr.M~~ # 3 skia.dll!SkScalerContext_GDI::generateImage                                [third_party\skia\src\ports\skfonthost_win.cpp:1233]
~~Dr.M~~ # 4 skia.dll!SkScalerContext::getImage                                         [third_party\skia\src\core\skscalercontext.cpp:530]
~~Dr.M~~ # 5 skia.dll!SkGlyphCache::OnceFillInImage                                     [third_party\skia\src\core\skglyphcache.cpp:252]
~~Dr.M~~ # 6 skia.dll!sk_once_slow<>                                                    [third_party\skia\include\core\skonce.h:76]
~~Dr.M~~ # 7 skia.dll!SkGlyphCache::findImage                                           [third_party\skia\src\core\skglyphcache.cpp:260]
~~Dr.M~~ # 8 skia.dll!D1G_RectClip                                                      [third_party\skia\src\core\skdraw.cpp:1479]
~~Dr.M~~ # 9 skia.dll!SkDraw::drawPosText                                               [third_party\skia\src\core\skdraw.cpp:1838]
~~Dr.M~~ #10 skia.dll!SkBitmapDevice::drawPosText                                       [third_party\skia\src\core\skbitmapdevice.cpp:348]
~~Dr.M~~ #11 skia.dll!SkCanvas::onDrawPosText                                           [third_party\skia\src\core\skcanvas.cpp:2433]
~~Dr.M~~ #12 skia.dll!SkCanvas::drawPosText                                             [third_party\skia\src\core\skcanvas.cpp:2507]
~~Dr.M~~ #13 skia.dll!SkRecords::Draw::draw<>                                           [third_party\skia\src\core\skrecorddraw.cpp:109]
~~Dr.M~~ #14 skia.dll!SkRecord::Record::visit<>                                         [third_party\skia\src\core\skrecord.h:170]
~~Dr.M~~ #15 skia.dll!SkRecordDraw                                                      [third_party\skia\src\core\skrecorddraw.cpp:55]
~~Dr.M~~ #16 skia.dll!SkBigPicture::playback                                            [third_party\skia\src\core\skbigpicture.cpp:43]
~~Dr.M~~ #17 skia.dll!SkCanvas::onDrawPicture                                           [third_party\skia\src\core\skcanvas.cpp:2800]
~~Dr.M~~ #18 skia.dll!SkCanvas::drawPicture                                             [third_party\skia\src\core\skcanvas.cpp:2770]
~~Dr.M~~ #19 cc.dll!cc::DrawingDisplayItem::Raster                                      [cc\playback\drawing_display_item.cc:51]
~~Dr.M~~ #20 cc.dll!cc::DisplayItemList::Raster                                         [cc\playback\display_item_list.cc:107]
~~Dr.M~~ #21 cc.dll!cc::DisplayListRasterSource::RasterCommon                           [cc\playback\display_list_raster_source.cc:122]
~~Dr.M~~ #22 cc.dll!cc::DisplayListRasterSource::PlaybackToCanvas                       [cc\playback\display_list_raster_source.cc:100]
~~Dr.M~~ #23 cc.dll!cc::TileTaskWorkerPool::PlaybackToMemory                            [cc\raster\tile_task_worker_pool.cc:208]
~~Dr.M~~ #24 cc.dll!cc::OneCopyTileTaskWorkerPool::PlaybackAndCopyOnWorkerThread        [cc\raster\one_copy_tile_task_worker_pool.cc:413]
~~Dr.M~~ #25 cc.dll!cc::`anonymous namespace'::RasterBufferImpl::Playback               [cc\raster\one_copy_tile_task_worker_pool.cc:53]
~~Dr.M~~ #26 cc.dll!cc::`anonymous namespace'::RasterTaskImpl::Raster                   [cc\tiles\tile_manager.cc:131]
~~Dr.M~~ #27 cc.dll!cc::`anonymous namespace'::RasterTaskImpl::RunOnWorkerThread        [cc\tiles\tile_manager.cc:90]
~~Dr.M~~ #28 cc.dll!cc::TaskGraphRunner::RunTaskWithLockAcquired                        [cc\raster\task_graph_runner.cc:418]
~~Dr.M~~ #29 cc.dll!cc::TaskGraphRunner::Run                                            [cc\raster\task_graph_runner.cc:361]
~~Dr.M~~ #30 base.dll!base::SimpleThread::ThreadMain                                    [base\threading\simple_thread.cc:66]
~~Dr.M~~ #31 base.dll!base::`anonymous namespace'::ThreadFunc                           [base\threading\platform_thread_win.cc:82]
~~Dr.M~~ #32 KERNEL32.dll!BaseThreadInitThunk                                          +0x11     (0x7570337a <KERNEL32.dll+0x1337a>)
~~Dr.M~~ Note: @0:15:51.087 in thread 196
~~Dr.M~~ Note: handles created with the same callstack are closed here:
~~Dr.M~~ Note: # 0 system call NtGdiDeleteObjectApp
~~Dr.M~~ Note: # 1 GDI32.dll!DeleteObject                                                    +0x149    (0x768e57d3 <GDI32.dll+0x157d3>)
~~Dr.M~~ Note: # 2 skia.dll!HDCOffscreen::draw                                                [third_party\skia\src\ports\skfonthost_win.cpp:471]
~~Dr.M~~ Note: # 3 skia.dll!SkScalerContext_GDI::generateImage                                [third_party\skia\src\ports\skfonthost_win.cpp:1233]
~~Dr.M~~ Note: # 4 skia.dll!SkScalerContext::getImage                                         [third_party\skia\src\core\skscalercontext.cpp:530]
~~Dr.M~~ Note: # 5 skia.dll!SkGlyphCache::OnceFillInImage                                     [third_party\skia\src\core\skglyphcache.cpp:252]
~~Dr.M~~ Note: # 6 skia.dll!sk_once_slow<>                                                    [third_party\skia\include\core\skonce.h:76]
~~Dr.M~~ Note: # 7 skia.dll!SkGlyphCache::findImage                                           [third_party\skia\src\core\skglyphcache.cpp:260]
~~Dr.M~~ Note: # 8 skia.dll!D1G_RectClip                                                      [third_party\skia\src\core\skdraw.cpp:1479]
~~Dr.M~~ Note: # 9 skia.dll!SkDraw::drawPosText                                               [third_party\skia\src\core\skdraw.cpp:1838]
~~Dr.M~~ Note: #10 skia.dll!SkBitmapDevice::drawPosText                                       [third_party\skia\src\core\skbitmapdevice.cpp:348]
~~Dr.M~~ Note: #11 skia.dll!SkCanvas::onDrawPosText                                           [third_party\skia\src\core\skcanvas.cpp:2433]
~~Dr.M~~ Note: #12 skia.dll!SkCanvas::drawPosText                                             [third_party\skia\src\core\skcanvas.cpp:2507]
~~Dr.M~~ Note: #13 skia.dll!SkRecords::Draw::draw<>                                           [third_party\skia\src\core\skrecorddraw.cpp:109]
~~Dr.M~~ Note: #14 skia.dll!SkRecord::Record::visit<>                                         [third_party\skia\src\core\skrecord.h:170]
~~Dr.M~~ Note: #15 skia.dll!SkRecordDraw                                                      [third_party\skia\src\core\skrecorddraw.cpp:55]
~~Dr.M~~ Note: #16 skia.dll!SkBigPicture::playback                                            [third_party\skia\src\core\skbigpicture.cpp:43]
~~Dr.M~~ Note: #17 skia.dll!SkCanvas::onDrawPicture                                           [third_party\skia\src\core\skcanvas.cpp:2800]
~~Dr.M~~ Note: #18 skia.dll!SkCanvas::drawPicture                                             [third_party\skia\src\core\skcanvas.cpp:2770]
~~Dr.M~~ Note: #19 cc.dll!cc::DrawingDisplayItem::Raster                                      [cc\playback\drawing_display_item.cc:51]
~~Dr.M~~ Note: #20 cc.dll!cc::DisplayItemList::Raster                                         [cc\playback\display_item_list.cc:107]
~~Dr.M~~ Note: #21 cc.dll!cc::DisplayListRasterSource::RasterCommon                           [cc\playback\display_list_raster_source.cc:122]
~~Dr.M~~ Note: #22 cc.dll!cc::DisplayListRasterSource::PlaybackToCanvas                       [cc\playback\display_list_raster_source.cc:100]
~~Dr.M~~ Note: #23 cc.dll!cc::TileTaskWorkerPool::PlaybackToMemory                            [cc\raster\tile_task_worker_pool.cc:208]
~~Dr.M~~ Note: #24 cc.dll!cc::OneCopyTileTaskWorkerPool::PlaybackAndCopyOnWorkerThread        [cc\raster\one_copy_tile_task_worker_pool.cc:413]
~~Dr.M~~ Note: #25 cc.dll!cc::`anonymous namespace'::RasterBufferImpl::Playback               [cc\raster\one_copy_tile_task_worker_pool.cc:53]
~~Dr.M~~ Note: #26 cc.dll!cc::`anonymous namespace'::RasterTaskImpl::Raster                   [cc\tiles\tile_manager.cc:131]
~~Dr.M~~ Note: #27 cc.dll!cc::`anonymous namespace'::RasterTaskImpl::RunOnWorkerThread        [cc\tiles\tile_manager.cc:90]
~~Dr.M~~ Note: #28 cc.dll!cc::TaskGraphRunner::RunTaskWithLockAcquired                        [cc\raster\task_graph_runner.cc:418]
~~Dr.M~~ Note: #29 cc.dll!cc::TaskGraphRunner::Run                                            [cc\raster\task_graph_runner.cc:361]
~~Dr.M~~ Note: #30 base.dll!base::SimpleThread::ThreadMain                                    [base\threading\simple_thread.cc:66]
~~Dr.M~~ Note: #31 base.dll!base::`anonymous namespace'::ThreadFunc                           [base\threading\platform_thread_win.cc:82]
~~Dr.M~~ Note: #32 KERNEL32.dll!BaseThreadInitThunk                                          +0x11     (0x7570337a <KERNEL32.dll+0x1337a>)

Original issue's description:
> Parallel cache.
>
> TBR=reed@google.com
>
> BUG=skia:1330
>
> Committed: https://skia.googlesource.com/skia/+/6f2a486040cb25465990196c229feb47e668e87f
>
> Committed: https://skia.googlesource.com/skia/+/bf2988833e5a36c6b430da6fdd2cfebd0015adec

TBR=reed@google.com,mtklein@google.com,mtklein@chromium.org,herb@google.com
BUG=skia:1330

[mtklein mucking around]
NOTREECHECKS=true

Review URL: https://codereview.chromium.org/1339493002

[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/include/core/SkAtomics.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/include/ports/SkAtomics_atomic.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/include/ports/SkAtomics_std.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/include/ports/SkAtomics_sync.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/src/core/SkDraw.cpp
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/src/core/SkGlyph.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/src/core/SkGlyphCache.cpp
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/src/core/SkGlyphCache.h
[modify] http://crrev.com/951d8543271ff541a45576bea207b33748802d2e/src/core/SkGlyphCache_Globals.h

Project Member Comment 8 by bugdroid1@chromium.org, Sep 15 2015
The following revision refers to this bug:
  https://skia.googlesource.com/skia.git/+/cd7f03597475ea423aa819bdae03996b26874dd5

commit cd7f03597475ea423aa819bdae03996b26874dd5
Author: herb <herb@google.com>
Date: Tue Sep 15 22:15:40 2015

Revert of Parallel cache - preliminary  (patchset #24 id:460001 of https://codereview.chromium.org/1264103003/ )

Reason for revert:
Breaks DrMemory in the chrome roll.

Original issue's description:
> Parallel cache.
>
> TBR=reed@google.com
>
> BUG=skia:1330,528560
>
> Committed: https://skia.googlesource.com/skia/+/6f2a486040cb25465990196c229feb47e668e87f
>
> Committed: https://skia.googlesource.com/skia/+/bf2988833e5a36c6b430da6fdd2cfebd0015adec
>
> Committed: https://skia.googlesource.com/skia/+/014ffdb01ea5317614a1569efc30c50f06434222

TBR=reed@google.com,mtklein@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:1330,528560

Review URL: https://codereview.chromium.org/1345903002

[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/include/core/SkAtomics.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/include/ports/SkAtomics_atomic.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/include/ports/SkAtomics_std.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/include/ports/SkAtomics_sync.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/src/core/SkDraw.cpp
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/src/core/SkGlyph.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/src/core/SkGlyphCache.cpp
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/src/core/SkGlyphCache.h
[modify] http://crrev.com/cd7f03597475ea423aa819bdae03996b26874dd5/src/core/SkGlyphCache_Globals.h

Sign in to add a comment