We now have jemalloc_ptr_info() and moz_malloc_enclosing_size_of(), which can be used to measure heap blocks via interior pointers. This patch does the following. - Adds MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF, for defining measure-via-interior-pointer functions. - Adds a mMallocEnclosingOf field to SizeOfState, to hold these functions. - Uses these functions to replace some horrid pointer arithmetic in functions measuring Rust types.

<none> This patch applies on top of the patch in bug 1389305.

Comment on attachment 8900638 [details] [diff] [review] Take advantage of new support for measuring heap blocks via interior pointers Review of attachment 8900638 [details] [diff] [review]: ----------------------------------------------------------------- > Adds a mMallocEnclosingOf field to SizeOfState, to hold these functions r-'ing for now pending clarification on this bit, otherwise looks good. ::: dom/base/nsWindowMemoryReporter.cpp @@ +51,5 @@ > AddNonJSSizeOfWindowAndItsDescendents(nsGlobalWindow* aWindow, > nsTabSizes* aSizes) > { > // Measure the window. > + SizeOfState state(moz_malloc_size_of, moz_malloc_enclosing_size_of); I wonder if we should just have a default version that uses the `moz_malloc` functions. ::: layout/style/ServoBindings.cpp @@ +246,5 @@ > + // servo_arc::Arc, i.e. it is preceded by a word-sized refcount. So we need > + // to measure it with a function that can handle an interior pointer. We use > + // ServoStyleStructsEnclosingMallocSizeOf rather than > + // |aState.mMallocEnclosingSizeOf| to better distinguish in DMD's output the > + // memory measured here. At this point why are we adding it to `SizeOfState` if we don't actually use it? ::: xpcom/base/SizeOfState.h @@ +57,5 @@ > class SizeOfState > { > public: > + explicit SizeOfState(MallocSizeOf aMallocSizeOf, > + MallocSizeOf aMallocEnclosingSizeOf) Might be worth having a version that doesn't require `aMallocEnclosingSizeOf`. For the 2 param version you probably don't need explicit anymore.

> > + SizeOfState state(moz_malloc_size_of, moz_malloc_enclosing_size_of); > > I wonder if we should just have a default version that uses the `moz_malloc` > functions. I prefer having explicit arguments. When I see moz_malloc_* being used I immediately know that the measurement is not for about:memory but for some other purpose (like estimating the size of a cache). > ::: layout/style/ServoBindings.cpp > @@ +246,5 @@ > > + // servo_arc::Arc, i.e. it is preceded by a word-sized refcount. So we need > > + // to measure it with a function that can handle an interior pointer. We use > > + // ServoStyleStructsEnclosingMallocSizeOf rather than > > + // |aState.mMallocEnclosingSizeOf| to better distinguish in DMD's output the > > + // memory measured here. > > At this point why are we adding it to `SizeOfState` if we don't actually use > it? Because it's the more general solution. Although the two places that might use mMallocEnclosingSizeOf currently don't, other places will in the future. E.g. there's a Rust-side version of SizeOfState that will use mMallocEnclosingSizeOf for bug 1387958. (You couldn't have known this, of course :) > > class SizeOfState > > { > > public: > > + explicit SizeOfState(MallocSizeOf aMallocSizeOf, > > + MallocSizeOf aMallocEnclosingSizeOf) > > Might be worth having a version that doesn't require > `aMallocEnclosingSizeOf`. Would that null out mMallocEnclosingSizeOf? I wrote it so both were required for simplicity, but I guess we could allow null there given that it's not used in lots of cases. But then it wouldn't be clear for any particular function that took a SizeOfState whether it needed the mMallocEnclosingSizeOf to be populated. Introducing a separate type would make it clear, but I'm not sure if that's overkill. > For the 2 param version you probably don't need explicit anymore. True.

Here is a smaller patch that omits the SizeOfState changes.

Comment on attachment 8901663 [details] [diff] [review] Take advantage of new support for measuring heap blocks via interior pointers Review of attachment 8901663 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks for narrowing it down for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b30f38210e155ef046b55ef22e19d771c13540e Bug 1393384 - Take advantage of new support for measuring heap blocks via interior pointers. r=erahm.

Backed out bug 1393384 and bug 1389305 for frequently failing (25% of the test runs) GTest Jemalloc.PtrInfo on Linux opt: https://hg.mozilla.org/integration/mozilla-inbound/rev/584579605f5f4e7f46434149f0c2e1e87cc49111 https://hg.mozilla.org/integration/mozilla-inbound/rev/0922fc9b3ae605ba93fa0036017c72afeb4e0f0a Push with failures https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5b30f38210e155ef046b55ef22e19d771c13540e&filter-searchStr=711f49d9771acf7fa2767abd6f05979f3a534a39 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=127368910&repo=mozilla-inbound [task 2017-08-31T11:36:30.809277Z] 11:36:30 INFO - TEST-START | Jemalloc.PtrInfo [task 2017-08-31T11:36:31.095072Z] 11:36:31 INFO - ExceptionHandler::GenerateDump cloned child 10221 [task 2017-08-31T11:36:31.095507Z] 11:36:31 INFO - ExceptionHandler::SendContinueSignalToChild sent continue signal to child [task 2017-08-31T11:36:31.095897Z] 11:36:31 INFO - ExceptionHandler::WaitForContinueSignal waiting for continue signal... [task 2017-08-31T11:36:31.479065Z] 11:36:31 INFO - mozcrash INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/MFuOeItiQBCW47agEur-jA/artifacts/public/build/target.crashreporter-symbols.zip [task 2017-08-31T11:36:38.507304Z] 11:36:38 INFO - mozcrash INFO | Copy/paste: /usr/local/bin/linux64-minidump_stackwalk /builds/worker/workspace/build/tests/gtest/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp /tmp/tmpVmJiqr [task 2017-08-31T11:36:47.021275Z] 11:36:47 INFO - mozcrash INFO | Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp [task 2017-08-31T11:36:47.021708Z] 11:36:47 INFO - mozcrash INFO | Saved app info as /builds/worker/workspace/build/blobber_upload_dir/3cbaba07-5872-23f8-0bb7-c6398552a86e.extra [task 2017-08-31T11:36:47.022171Z] 11:36:47 WARNING - PROCESS-CRASH | gtest | application crashed [@ Jemalloc_PtrInfo_Test::TestBody] [task 2017-08-31T11:36:47.022800Z] 11:36:47 INFO - Crash dump filename: /builds/worker/workspace/build/tests/gtest/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp [task 2017-08-31T11:36:47.022917Z] 11:36:47 INFO - Operating system: Linux [task 2017-08-31T11:36:47.023444Z] 11:36:47 INFO - 0.0.0 Linux 3.13.0-112-generic #159-Ubuntu SMP Fri Mar 3 15:26:07 UTC 2017 x86_64 [task 2017-08-31T11:36:47.023881Z] 11:36:47 INFO - CPU: x86 [task 2017-08-31T11:36:47.024329Z] 11:36:47 INFO - GenuineIntel family 6 model 62 stepping 4 [task 2017-08-31T11:36:47.024786Z] 11:36:47 INFO - 4 CPUs [task 2017-08-31T11:36:47.025202Z] 11:36:47 INFO - GPU: UNKNOWN [task 2017-08-31T11:36:47.025590Z] 11:36:47 INFO - Crash reason: SIGFPE [task 2017-08-31T11:36:47.026033Z] 11:36:47 INFO - Crash address: 0xf30a3c91 [task 2017-08-31T11:36:47.026482Z] 11:36:47 INFO - Process uptime: not available [task 2017-08-31T11:36:47.026942Z] 11:36:47 INFO - Thread 0 (crashed) [task 2017-08-31T11:36:47.027290Z] 11:36:47 INFO - 0 libxul.so!Jemalloc_PtrInfo_Test::TestBody [TestJemalloc.cpp:5b30f38210e1 : 143 + 0x7] [task 2017-08-31T11:36:47.027770Z] 11:36:47 INFO - eip = 0xf30a3c91 esp = 0xffa39c20 ebp = 0xffa39cf8 ebx = 0xf5433000 [task 2017-08-31T11:36:47.028262Z] 11:36:47 INFO - esi = 0xb8f0b800 edi = 0x00000101 eax = 0x000541fc ecx = 0x00000101 [task 2017-08-31T11:36:47.028708Z] 11:36:47 INFO - edx = 0x00000000 efl = 0x00010246 [task 2017-08-31T11:36:47.029178Z] 11:36:47 INFO - Found by: given as instruction pointer in context [task 2017-08-31T11:36:47.029629Z] 11:36:47 INFO - 1 libxul.so!testing::Test::Run [gtest.cc:5b30f38210e1 : 2475 + 0x21] [task 2017-08-31T11:36:47.030130Z] 11:36:47 INFO - eip = 0xf2d54cd9 esp = 0xffa39d00 ebp = 0xffa39d38 ebx = 0xf5433000 [task 2017-08-31T11:36:47.030578Z] 11:36:47 INFO - esi = 0x9a272180 edi = 0xf7159150 [task 2017-08-31T11:36:47.030961Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.031409Z] 11:36:47 INFO - 2 libxul.so!testing::TestInfo::Run [gtest.cc:5b30f38210e1 : 2466 + 0x1a] [task 2017-08-31T11:36:47.032020Z] 11:36:47 INFO - eip = 0xf2d5d2a5 esp = 0xffa39d40 ebp = 0xffa39d98 ebx = 0xf5433000 [task 2017-08-31T11:36:47.032249Z] 11:36:47 INFO - esi = 0xeea6e4e0 edi = 0xf7159150 [task 2017-08-31T11:36:47.032653Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.032967Z] 11:36:47 INFO - 3 libxul.so!testing::TestCase::Run [gtest.cc:5b30f38210e1 : 2631 + 0x12] [task 2017-08-31T11:36:47.033406Z] 11:36:47 INFO - eip = 0xf2d5d44d esp = 0xffa39da0 ebp = 0xffa39e08 ebx = 0x00000001 [task 2017-08-31T11:36:47.033708Z] 11:36:47 INFO - esi = 0xeea69b00 edi = 0xf7127690 [task 2017-08-31T11:36:47.034242Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.035034Z] 11:36:47 INFO - 4 libxul.so!testing::internal::UnitTestImpl::RunAllTests [gtest.cc:5b30f38210e1 : 4687 + 0x18] [task 2017-08-31T11:36:47.035121Z] 11:36:47 INFO - eip = 0xf2d5d92d esp = 0xffa39e10 ebp = 0xffa39e98 ebx = 0x000000fc [task 2017-08-31T11:36:47.035186Z] 11:36:47 INFO - esi = 0xf7159150 edi = 0xf716c4b8 [task 2017-08-31T11:36:47.035638Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.036094Z] 11:36:47 INFO - 5 libxul.so!testing::UnitTest::Run [gtest.cc:5b30f38210e1 : 2458 + 0x8] [task 2017-08-31T11:36:47.036620Z] 11:36:47 INFO - eip = 0xf2d5dc74 esp = 0xffa39ea0 ebp = 0xffa39ec8 ebx = 0xf5433000 [task 2017-08-31T11:36:47.036948Z] 11:36:47 INFO - esi = 0xf7159150 edi = 0xef3878c0 [task 2017-08-31T11:36:47.037355Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.037787Z] 11:36:47 INFO - 6 libxul.so!mozilla::RunGTestFunc [gtest.h:5b30f38210e1 : 2233 + 0xd] [task 2017-08-31T11:36:47.038178Z] 11:36:47 INFO - eip = 0xf2d603f5 esp = 0xffa39ed0 ebp = 0xffa39f38 ebx = 0xf5433000 [task 2017-08-31T11:36:47.038548Z] 11:36:47 INFO - esi = 0xede7afd4 edi = 0xef3878c0 [task 2017-08-31T11:36:47.038976Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.039516Z] 11:36:47 INFO - 7 libxul.so!XREMain::XRE_mainStartup [nsAppRunner.cpp:5b30f38210e1 : 3877 + 0x9] [task 2017-08-31T11:36:47.039798Z] 11:36:47 INFO - eip = 0xf280f634 esp = 0xffa39f40 ebp = 0xffa3a118 ebx = 0xf5433000 [task 2017-08-31T11:36:47.040180Z] 11:36:47 INFO - esi = 0xffa3a080 edi = 0xffa3a06c [task 2017-08-31T11:36:47.040646Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.041110Z] 11:36:47 INFO - 8 libxul.so!XREMain::XRE_main [nsAppRunner.cpp:5b30f38210e1 : 4794 + 0xf] [task 2017-08-31T11:36:47.041532Z] 11:36:47 INFO - eip = 0xf28139ff esp = 0xffa3a120 ebp = 0xffa3a178 ebx = 0xf5433000 [task 2017-08-31T11:36:47.042005Z] 11:36:47 INFO - esi = 0x00000000 edi = 0xffa3a154 [task 2017-08-31T11:36:47.042478Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.042857Z] 11:36:47 INFO - 9 libxul.so!XRE_main [nsAppRunner.cpp:5b30f38210e1 : 4904 + 0x5] [task 2017-08-31T11:36:47.043352Z] 11:36:47 INFO - eip = 0xf2813ea7 esp = 0xffa3a180 ebp = 0xffa3a2d8 ebx = 0x08075000 [task 2017-08-31T11:36:47.043735Z] 11:36:47 INFO - esi = 0xffa3a1a4 edi = 0xffa3a1e8 [task 2017-08-31T11:36:47.044097Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.044541Z] 11:36:47 INFO - 10 firefox!do_main [nsBrowserApp.cpp:5b30f38210e1 : 236 + 0x1a] [task 2017-08-31T11:36:47.044994Z] 11:36:47 INFO - eip = 0x0804d83a esp = 0xffa3a2e0 ebp = 0xffa3b328 ebx = 0x08075000 [task 2017-08-31T11:36:47.045326Z] 11:36:47 INFO - esi = 0xffa3b424 edi = 0x00000002 [task 2017-08-31T11:36:47.045780Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.046222Z] 11:36:47 INFO - 11 firefox!main [nsBrowserApp.cpp:5b30f38210e1 : 309 + 0xc] [task 2017-08-31T11:36:47.046681Z] 11:36:47 INFO - eip = 0x0804cf81 esp = 0xffa3b330 ebp = 0xffa3b378 ebx = 0x08075000 [task 2017-08-31T11:36:47.047147Z] 11:36:47 INFO - esi = 0xffa3b424 edi = 0x00000002 [task 2017-08-31T11:36:47.047583Z] 11:36:47 INFO - Found by: call frame info [task 2017-08-31T11:36:47.048092Z] 11:36:47 INFO - 12 libc-2.23.so + 0x18637 [task 2017-08-31T11:36:47.048537Z] 11:36:47 INFO - eip = 0xf734d637 esp = 0xffa3b380 ebp = 0x00000000 [task 2017-08-31T11:36:47.049065Z] 11:36:47 INFO - Found by: previous frame's frame pointer [task 2017-08-31T11:36:47.049580Z] 11:36:47 INFO - 13 libc-2.23.so + 0x1b2000 [task 2017-08-31T11:36:47.050148Z] 11:36:47 INFO - eip = 0xf74e7000 esp = 0xffa3b384 ebp = 0x00000000 [task 2017-08-31T11:36:47.050298Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.050752Z] 11:36:47 INFO - 14 libc-2.23.so + 0x1b2000 [task 2017-08-31T11:36:47.051308Z] 11:36:47 INFO - eip = 0xf74e7000 esp = 0xffa3b388 ebp = 0x00000000 [task 2017-08-31T11:36:47.051512Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.052128Z] 11:36:47 INFO - 15 libc-2.23.so + 0x18637 [task 2017-08-31T11:36:47.052485Z] 11:36:47 INFO - eip = 0xf734d637 esp = 0xffa3b390 ebp = 0x00000000 [task 2017-08-31T11:36:47.052786Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.053361Z] 11:36:47 INFO - 16 libc-2.23.so + 0x1b2000 [task 2017-08-31T11:36:47.056094Z] 11:36:47 INFO - eip = 0xf74e7000 esp = 0xffa3b3ac ebp = 0x00000000 [task 2017-08-31T11:36:47.056142Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.056180Z] 11:36:47 INFO - 17 libc-2.23.so + 0x1b2000 [task 2017-08-31T11:36:47.056748Z] 11:36:47 INFO - eip = 0xf74e7000 esp = 0xffa3b3bc ebp = 0x00000000 [task 2017-08-31T11:36:47.056804Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.056851Z] 11:36:47 INFO - 18 libc-2.23.so + 0x1b2000 [task 2017-08-31T11:36:47.057162Z] 11:36:47 INFO - eip = 0xf74e7000 esp = 0xffa3b3c0 ebp = 0x00000000 [task 2017-08-31T11:36:47.057212Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.057734Z] 11:36:47 INFO - 19 firefox + 0x5264 [task 2017-08-31T11:36:47.058017Z] 11:36:47 INFO - eip = 0x0804d264 esp = 0xffa3b3e0 ebp = 0x00000000 [task 2017-08-31T11:36:47.058306Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.058770Z] 11:36:47 INFO - 20 ld-2.23.so + 0x15030 [task 2017-08-31T11:36:47.059288Z] 11:36:47 INFO - eip = 0xf773a030 esp = 0xffa3b3e8 ebp = 0x00000000 [task 2017-08-31T11:36:47.059528Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.060106Z] 11:36:47 INFO - 21 ld-2.23.so + 0xf8a0 [task 2017-08-31T11:36:47.060555Z] 11:36:47 INFO - eip = 0xf77348a0 esp = 0xffa3b3ec ebp = 0x00000000 [task 2017-08-31T11:36:47.061148Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.061326Z] 11:36:47 INFO - 22 firefox + 0x5264 [task 2017-08-31T11:36:47.061762Z] 11:36:47 INFO - eip = 0x0804d264 esp = 0xffa3b3f8 ebp = 0x00000000 [task 2017-08-31T11:36:47.062276Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.062718Z] 11:36:47 INFO - 23 firefox!_start + 0x21 [task 2017-08-31T11:36:47.063146Z] 11:36:47 INFO - eip = 0x0804d285 esp = 0xffa3b400 ebp = 0x00000000 [task 2017-08-31T11:36:47.063601Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.064111Z] 11:36:47 INFO - 24 firefox + 0x4ed0 [task 2017-08-31T11:36:47.064374Z] 11:36:47 INFO - eip = 0x0804ced0 esp = 0xffa3b404 ebp = 0x00000000 [task 2017-08-31T11:36:47.064918Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.065278Z] 11:36:47 INFO - 25 firefox!__libc_csu_fini + 0x10 [task 2017-08-31T11:36:47.065856Z] 11:36:47 INFO - eip = 0x0806c470 esp = 0xffa3b410 ebp = 0xffa3b424 [task 2017-08-31T11:36:47.066051Z] 11:36:47 INFO - Found by: stack scanning [task 2017-08-31T11:36:47.066428Z] 11:36:47 INFO - 26 0xffa3c693 [task 2017-08-31T11:36:47.066972Z] 11:36:47 INFO - eip = 0xffa3c693 esp = 0xffa3b42c ebp = 0xffa3c658 [task 2017-08-31T11:36:47.067089Z] 11:36:47 INFO - Found by: previous frame's frame pointer [task 2017-08-31T11:36:47.067477Z] 11:36:47 INFO - 27 0x2f73646c [task 2017-08-31T11:36:47.067929Z] 11:36:47 INFO - eip = 0x2f73646c esp = 0xffa3c660 ebp = 0x6975622f [task 2017-08-31T11:36:47.068156Z] 11:36:47 INFO - Found by: previous frame's frame pointer

I just relanded a tweaked version of bug 1389305. Once it looks stable I will reland this patch unchanged.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe1ff5e55f1ff8ca8f868047b714ab69bf35d690 Bug 1393384 (attempt 2) - Take advantage of new support for measuring heap blocks via interior pointers. r=erahm.

Pushed by nnethercote@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe1ff5e55f1f (attempt 2) - Take advantage of new support for measuring heap blocks via interior pointers. r=erahm.