Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1264103003:
Parallel cache - preliminary (Closed)
DescriptionParallel 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
Patch Set 1 #Patch Set 2 : glyphcache-uses-shared-locks #Patch Set 3 : working-but-no-purge #Patch Set 4 : fix ws #Patch Set 5 : more ws fixes #Patch Set 6 : even more ws fixes #Patch Set 7 : fully working #
Total comments: 14
Patch Set 8 : Address first round of comments #Patch Set 9 : fix windows compile problems #Patch Set 10 : fix windows atomics #Patch Set 11 : Use SkOnce for metrics #Patch Set 12 : Fix race in SkDraw #Patch Set 13 : Add fence for reads to bitmap #Patch Set 14 : Fix full metrics in allocate #Patch Set 15 : move metrics update into mutex #Patch Set 16 : Add barrier to metrics data. #Patch Set 17 : add atomic to isJustify #Patch Set 18 : only metrics no advances #Patch Set 19 : advances state removed #Patch Set 20 : remove atomics from skglyph #
Total comments: 6
Patch Set 21 : address mtklein comments #Patch Set 22 : round of cleanup nullptr sknew #Patch Set 23 : reapply change #Patch Set 24 : use after free fixed #Patch Set 25 : parallel cache 2 #
Messages
Total messages: 59 (23 generated)
herb@google.com changed reviewers: + mtklein@google.com, reed@google.com
This is close to the final form. It has good performance characteristics for nytimes, but I need to add the cache purging code back in and run perf again. It is multi-threaded, and only one version of each strike exists. This is basically to get a taste of how the code is shaping up.
I will publish preliminary performance info in an email because of bad formatting here.
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/120001
The CQ bit was unchecked by mtklein@chromium.org
https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.cpp File src/core/SkGlyphCache.cpp (right): https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:86: fMu.releaseShared(); Let's add assertShared() / assertExclusive() methods to SkSharedMutex. It'll help document the invariants in these methods, which are somewhat complicated now. https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:140: SkAutoMutexAcquire lock(fScalerMutex); Is this different from SkAutoMutexAcquire lock(fScalerMutex); return fScalerContext->glyphIDToChar(glyphID); ? https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:201: addFullMetrics(glyph); this-> https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:213: SkAutoTAcquire<SkSharedMutex> mapLock(fMu); Let's find some common naming scheme between fMu and fScalerMutex. I like fScalerMutex. fMu is a little vague. https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:226: increaseMemoryUsed(sizeof(SkGlyph)); this-> https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:252: void SkGlyphCache::onceFillInImage(GlyphAndCache gc) { It'd be nice to remind ourselves we're holding the scaler mutex in these methods with fScalerMutex.assertHeld(); https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:257: sk_atomic_store(&const_cast<SkGlyph*>(glyph)->fImage, This shouldn't need to be atomic under this scheme, called by SkOnce. https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:269: return sk_atomic_load(&glyph.fImage, sk_memory_order_relaxed); Again, I think these do not need to be atomic. https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:276: sk_atomic_store(&const_cast<SkGlyph*>(glyph)->fPath, SkNEW(SkPath), sk_memory_order_relaxed); ditto https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:285: return sk_atomic_load(&glyph.fPath, sk_memory_order_relaxed); ditto https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:310: SkAutoMutexAcquire lock(fScalerMutex); It's not obvious why these hold fScalerMutex? https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:421: cache->refCount += 1; This seems odd. Are all these cache->refCounts always guarded by globals.fLock? If so, let's document / assert the crap out of it. Locks spread out over two different objects are confusing and easily screwed up. We might want to brainstorm a bit about a different name, just given how specific a meaning ref count has in the rest of Skia. https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.h File src/core/SkGlyphCache.h (right): https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.h:10: #include <SkSpinlock.h> kill?
https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.cpp File src/core/SkGlyphCache.cpp (right): https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... src/core/SkGlyphCache.cpp:269: return sk_atomic_load(&glyph.fImage, sk_memory_order_relaxed); On 2015/08/14 15:31:37, mtklein_C wrote: > Again, I think these do not need to be atomic. I don't think this is correct. I think that making the load of glyph.fImage non-atomic, means that there is no happens-before relationship between the mutex and this variable. So, I think that the change would be a data race (1.10.21) because glyph.fImage is non-atomic and there is no happens-before relationship because the mutex does not synchronize with the fImage field. By making the loads atomic relaxed forces the fImage read to not be a data race. From ISO: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...) Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
On 2015/08/21 19:16:50, herb_g wrote: > https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.cpp > File src/core/SkGlyphCache.cpp (right): > > https://codereview.chromium.org/1264103003/diff/120001/src/core/SkGlyphCache.... > src/core/SkGlyphCache.cpp:269: return sk_atomic_load(&glyph.fImage, > sk_memory_order_relaxed); > On 2015/08/14 15:31:37, mtklein_C wrote: > > Again, I think these do not need to be atomic. > > I don't think this is correct. I think that making the load of glyph.fImage > non-atomic, means that there is no happens-before relationship between the mutex > and this variable. So, I think that the change would be a data race (1.10.21) > because glyph.fImage is non-atomic and there is no happens-before relationship > because the mutex does not synchronize with the fImage field. By making the > loads atomic relaxed forces the fImage read to not be a data race. > > From ISO: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf > The execution of a program contains a data race if it contains two conflicting > actions in different threads, > at least one of which is not atomic, and neither happens before the other. Any > such data race results in > undefined behavior OK, I'm still confused but more atomic never hurts. Can you add a note to remind me/us to circle back on this question later?
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/360001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/360001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...) Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/380001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/380001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Ok. This one is looking pretty good. The TSAN and ASAN pass. Performance numbers to follow... The
Oops. too soon. The two methods findImage and findPath use a strong barrier. I will experiment with what is going on there. The problem with find image is that a thread was reading before it was done filling in the image bytes. According to my understanding this should only need to be relaxed.
lgtm, nits only https://codereview.chromium.org/1264103003/diff/380001/include/core/SkAtomics.h File include/core/SkAtomics.h (right): https://codereview.chromium.org/1264103003/diff/380001/include/core/SkAtomics... include/core/SkAtomics.h:31: Can you declare sk_atomic_fetch_sub() here for documentation? https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.h File src/core/SkGlyphCache.h (right): https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.... src/core/SkGlyphCache.h:130: static void onceFillInMetrics(GlyphAndCache); These static members are NamedLikeThis: OnceFillInMetrics, OnceFillInImage, etc. https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.... src/core/SkGlyphCache.h:238: // Return a new SkGlyph for the glyph ID and subpixel position id. Limit the amount Kill "limit the amount of work using type" :)
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/420001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/420001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The tsan problem is the well know: SkPathPriv::CheapComputeFirstDirection https://codereview.chromium.org/1264103003/diff/380001/include/core/SkAtomics.h File include/core/SkAtomics.h (right): https://codereview.chromium.org/1264103003/diff/380001/include/core/SkAtomics... include/core/SkAtomics.h:31: On 2015/09/03 21:16:08, mtklein_C wrote: > Can you declare sk_atomic_fetch_sub() here for documentation? Done. https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.h File src/core/SkGlyphCache.h (right): https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.... src/core/SkGlyphCache.h:130: static void onceFillInMetrics(GlyphAndCache); On 2015/09/03 21:16:08, mtklein_C wrote: > These static members are NamedLikeThis: OnceFillInMetrics, OnceFillInImage, etc. Done. https://codereview.chromium.org/1264103003/diff/380001/src/core/SkGlyphCache.... src/core/SkGlyphCache.h:238: // Return a new SkGlyph for the glyph ID and subpixel position id. Limit the amount On 2015/09/03 21:16:08, mtklein_C wrote: > Kill "limit the amount of work using type" :) Done.
lgtm go go go!
The CQ bit was checked by herb@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@chromium.org Link to the patchset: https://codereview.chromium.org/1264103003/#ps420001 (title: "round of cleanup nullptr sknew")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/420001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/420001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)
This is one of those cases where it's fine to TBR=reed@google.com this. The files changed in include/ are not part of the public API. They merely predate include/private.
The CQ bit was checked by herb@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/420001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/420001
Message was sent while issue was closed.
Committed patchset #22 (id:420001) as https://skia.googlesource.com/skia/+/6f2a486040cb25465990196c229feb47e668e87f
Message was sent while issue was closed.
A revert of this CL (patchset #22 id:420001) has been created in https://codereview.chromium.org/1327703003/ by herb@google.com. The reason for reverting is: Seems to freeze android devices..
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/440001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/440001
Let's try this again.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by herb@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com, mtklein@chromium.org Link to the patchset: https://codereview.chromium.org/1264103003/#ps440001 (title: "reapply change")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/440001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/440001
Message was sent while issue was closed.
Committed patchset #23 (id:440001) as https://skia.googlesource.com/skia/+/bf2988833e5a36c6b430da6fdd2cfebd0015adec
Message was sent while issue was closed.
A revert of this CL (patchset #23 id:440001) has been created in https://codereview.chromium.org/1339493002/ by jyasskin@chromium.org. The reason for reverting is: Appears to leak GDI handles: http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28... ~~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>) .
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/460001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/460001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by herb@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com, mtklein@chromium.org Link to the patchset: https://codereview.chromium.org/1264103003/#ps460001 (title: "use after free fixed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264103003/460001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264103003/460001
Message was sent while issue was closed.
Committed patchset #24 (id:460001) as https://skia.googlesource.com/skia/+/014ffdb01ea5317614a1569efc30c50f06434222
Message was sent while issue was closed.
A revert of this CL (patchset #24 id:460001) has been created in https://codereview.chromium.org/1345903002/ by herb@google.com. The reason for reverting is: Breaks DrMemory in the chrome roll. . |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
