From e622b5e3e8590c4cae005c57cc225dd4f0cdb397 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Fri, 31 Jul 2015 22:43:24 -0400 Subject: [PATCH] Bug 1132966 - use relaxed Atomic integers for tracking graphics surface memory usage; r=njn Graphics surface memory usage tracking is done manually, with a global array containing the number of bytes per each type of surface used. Since the members of the array can be touched by several different threads, dynamic race checkers such as TSan complain about To assuage TSan's sensibilities, we need to use atomics with relaxed memory consistency; this change generates code identical to what we had before, but the atomic type assures TSan that it's OK to access members on multiple threads. We use the relaxed memory consistency to avoid memory barriers in the generated code. --- gfx/thebes/gfxASurface.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/gfx/thebes/gfxASurface.cpp b/gfx/thebes/gfxASurface.cpp index 07d4739adfc3..78db721361d4 100644 --- a/gfx/thebes/gfxASurface.cpp +++ b/gfx/thebes/gfxASurface.cpp @@ -564,13 +564,25 @@ PR_STATIC_ASSERT(uint32_t(CAIRO_SURFACE_TYPE_SKIA) == /* Surface size memory reporting */ -static int64_t gSurfaceMemoryUsed[size_t(gfxSurfaceType::Max)] = { 0 }; - class SurfaceMemoryReporter final : public nsIMemoryReporter { ~SurfaceMemoryReporter() {} + // We can touch this array on several different threads, and we don't + // want to introduce memory barriers when recording the memory used. To + // assure dynamic race checkers like TSan that this is OK, we use + // relaxed memory ordering here. + static Atomic sSurfaceMemoryUsed[size_t(gfxSurfaceType::Max)]; + public: + static void AdjustUsedMemory(gfxSurfaceType aType, int32_t aBytes) + { + // A read-modify-write operation like += would require a memory barrier + // here, which would defeat the purpose of using relaxed memory + // ordering. So separate out the read and write operations. + sSurfaceMemoryUsed[size_t(aType)] = sSurfaceMemoryUsed[size_t(aType)] + aBytes; + }; + NS_DECL_ISUPPORTS NS_IMETHOD CollectReports(nsIMemoryReporterCallback *aCb, @@ -578,7 +590,7 @@ public: { const size_t len = ArrayLength(sSurfaceMemoryReporterAttrs); for (size_t i = 0; i < len; i++) { - int64_t amount = gSurfaceMemoryUsed[i]; + int64_t amount = sSurfaceMemoryUsed[i]; if (amount != 0) { const char *path = sSurfaceMemoryReporterAttrs[i].path; @@ -589,7 +601,7 @@ public: nsresult rv = aCb->Callback(EmptyCString(), nsCString(path), KIND_OTHER, UNITS_BYTES, - gSurfaceMemoryUsed[i], + amount, nsCString(desc), aClosure); NS_ENSURE_SUCCESS(rv, rv); } @@ -599,6 +611,8 @@ public: } }; +Atomic SurfaceMemoryReporter::sSurfaceMemoryUsed[size_t(gfxSurfaceType::Max)]; + NS_IMPL_ISUPPORTS(SurfaceMemoryReporter, nsIMemoryReporter) void @@ -616,7 +630,7 @@ gfxASurface::RecordMemoryUsedForSurfaceType(gfxSurfaceType aType, registered = true; } - gSurfaceMemoryUsed[size_t(aType)] += aBytes; + SurfaceMemoryReporter::AdjustUsedMemory(aType, aBytes); } void