811018 - DMD flags double-report of images on B2G

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Justin Lebar (not reading bugmail)]

From the main B2G process on desktop Linux:

==16670== Double report of heap block 0x29EE3330:
==16670==  Allocated
==16670==    at 0x402B2AF: malloc (vg_replace_malloc.c:270)
==16670==    by 0x68545E7: imgFrame::Init(int, int, int, int, gfxASurface::gfxImageFormat, unsigned char) (imgFrame.cpp:170)
==16670==    by 0x684B3DA: mozilla::image::RasterImage::InternalAddFrame(unsigned int, int, int, int, int, gfxASurface::gfxImageFormat, unsigned char, unsigned char**, unsigned int*, unsigned int**, unsigned int*) (RasterImage.cpp:1253)
==16670==    by 0x684B6A3: mozilla::image::RasterImage::EnsureFrame(unsigned int, int, int, int, int, gfxASurface::gfxImageFormat, unsigned char, unsigned char**, unsigned int*, unsigned int**, unsigned int*) (RasterImage.cpp:1375)
==16670==    by 0x686A91E: mozilla::image::nsGIFDecoder2::BeginImageFrame(unsigned short) (nsGIFDecoder2.cpp:185)
==16670==    by 0x686B240: mozilla::image::nsGIFDecoder2::WriteInternal(char const*, unsigned int) (nsGIFDecoder2.cpp:956)
==16670==    by 0x684C0D8: mozilla::image::RasterImage::WriteToDecoder(char const*, unsigned int) (RasterImage.cpp:2625)
==16670==    by 0x684E3BD: mozilla::image::RasterImage::DecodeSomeData(unsigned int) (RasterImage.cpp:3141)
==16670==  Previously reported by 'images'
==16670==    at 0x685F06E: ImagesMallocSizeOf(void const*) (imgLoader.cpp:66)
==16670==    by 0x6855BCD: imgFrame::SizeOfExcludingThisWithComputedFallbackIfHeap(gfxASurface::MemoryLocation, unsigned long (*)(void const*)) const (imgFrame.cpp:806)
==16670==    by 0x684B132: mozilla::image::RasterImage::SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation, unsigned long (*)(void const*)) const (RasterImage.cpp:1166)
==16670==    by 0x685EFD3: imgMemoryReporter::EntryUsedUncompressedSize(nsACString_internal const&, imgCacheEntry*, void*) (imgLoader.cpp:252)
==16670==    by 0x685F0C1: nsBaseHashtable<nsCStringHashKey, nsRefPtr<imgCacheEntry>, imgCacheEntry*>::s_EnumReadStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsBaseHashtable.h:400)
==16670==    by 0x750E80F: PL_DHashTableEnumerate (pldhash.cpp:717)
==16670==    by 0x685EF5E: nsBaseHashtable<nsCStringHashKey, nsRefPtr<imgCacheEntry>, imgCacheEntry*>::EnumerateRead(PLDHashOperator (*)(nsACString_internal const&, imgCacheEntry*, void*), void*) const (nsBaseHashtable.h:188)
==16670==    by 0x685EEFF: imgMemoryReporter::GetImagesContentUsedUncompressed() (imgLoader.cpp:172)
==16670==  Now reported by 'images'
==16670==    at 0x685F06E: ImagesMallocSizeOf(void const*) (imgLoader.cpp:66)
==16670==    by 0x6855BCD: imgFrame::SizeOfExcludingThisWithComputedFallbackIfHeap(gfxASurface::MemoryLocation, unsigned long (*)(void const*)) const (imgFrame.cpp:806)
==16670==    by 0x684B132: mozilla::image::RasterImage::SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation, unsigned long (*)(void const*)) const (RasterImage.cpp:1166)
==16670==    by 0x685F23D: imgMemoryReporter::EntryAllSizes(nsACString_internal const&, imgCacheEntry*, void*) (imgLoader.cpp:221)
==16670==    by 0x685F0C1: nsBaseHashtable<nsCStringHashKey, nsRefPtr<imgCacheEntry>, imgCacheEntry*>::s_EnumReadStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsBaseHashtable.h:400)
==16670==    by 0x750E80F: PL_DHashTableEnumerate (pldhash.cpp:717)
==16670==    by 0x685EF5E: nsBaseHashtable<nsCStringHashKey, nsRefPtr<imgCacheEntry>, imgCacheEntry*>::EnumerateRead(PLDHashOperator (*)(nsACString_internal const&, imgCacheEntry*, void*), void*) const (nsBaseHashtable.h:188)
==16670==    by 0x685C8B6: imgMemoryReporter::CollectReports(nsIMemoryMultiReporterCallback*, nsISupports*) (imgLoader.cpp:92)

Comment 1

[Justin Lebar (not reading bugmail)]

Do we mark double-reporting bugs as blocking darkmatter?

Comment 2

[Nicholas Nethercote [inactive]]

The problem here is that we have a "images-content-used-uncompressed" single reporter, which is present just for telemetry purposes.  It overlaps with "explicit/images/content/used/uncompressed-heap". 

So DMD is right that some heap blocks are double-measured, but it's ok in this case because they don't both appear in the "explicit" tree.

Probably the best fix is to change DMDVCheckAndDump() to skip memory reporters that aren't in the "explicit" tree.  But I'm also wondering why I haven't seen this.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Avoid image double-counting false positive from DMD.

This should fix it, though I wasn't able to replicate the original problem.

Comment 4

[Justin Lebar (not reading bugmail)]

Comment on attachment 680915 [details] [diff] [review]
Avoid image double-counting false positive from DMD.

Do you want to change the multi-reporters as well?

Comment 5

[Nicholas Nethercote [inactive]]

> Do you want to change the multi-reporters as well?

It's not possible -- for single reporters you can inspect the other properties (path, kind) before requesting the amount, but you can't do that for multi-reporters.  And it's not needed for this case and any likely similar cases.

Comment 6

[Justin Lebar (not reading bugmail)]

> It's not possible -- for single reporters you can inspect the other properties (path, kind) before 
> requesting the amount, but you can't do that for multi-reporters.

Ah, right!

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b88cc4685734

Comment 8

[Nicholas Nethercote [inactive]]

I encountered this earlier today with an old trunk build;  updating fixed it.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b88cc4685734