I found that class name could be calculated automatically even if we have no-rtti. patch to follow.
Created attachment 183434 [details] Patch
Comment on attachment 183434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183434&action=review How does it affect the binary size? > Source/WTF/wtf/MemoryInstrumentation.cpp:140 > + buffer[length] = 0; You may be writing to buffer[maxLength] here which is out of the array bounds.
Created attachment 183437 [details] Patch
comments addressed. the footprint is about 174240 bytes. 0.1% of binary size.
Comment on attachment 183437 [details] Patch Attachment 183437 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15938657
(In reply to comment #4) > comments addressed. > > the footprint is about 174240 bytes. 0.1% of binary size. We may want put this behind a preprocessor guard to avoid the overhead when the data is not used.
Comment on attachment 183437 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183437&action=review > Source/WTF/ChangeLog:6 > + Reviewed by NOBODY (OOPS!). Please add an explanation on why need this change.
Comment on attachment 183437 [details] Patch Attachment 183437 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15947452 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Created attachment 183458 [details] Patch
Created attachment 183465 [details] Patch
Comment on attachment 183465 [details] Patch Attachment 183465 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15945575
Comment on attachment 183465 [details] Patch Attachment 183465 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15939667 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Created attachment 183483 [details] screenshot of Native Memory Snapshot for Angry Birds The final version looks like this. Interesting facts: Instrumentation collects data about 76MB of memory. It is not included V8 memory. About half of that memory is used for audio Head-related transfer function. Inspector may affect inspected page memory with his overlay page which is used by Elements panel. It uses about 13 MB for images.
It doesn't include V8 memory _yet_.
Created attachment 183485 [details] Patch
Created attachment 183486 [details] Patch
Comment on attachment 183486 [details] Patch Attachment 183486 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15938769 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Comment on attachment 183486 [details] Patch Attachment 183486 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15938793 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Comment on attachment 183486 [details] Patch Attachment 183486 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15940875 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
(In reply to comment #4) > comments addressed. > > the footprint is about 174240 bytes. 0.1% of binary size. How come it is way bigger than the size of all edge names which according to https://bugs.webkit.org/show_bug.cgi?id=107369#c14 increased the binary size just by 20120 bytes. Given that there are more instrumented fields than instrumented classes I believe we can reduce the overhead.
Created attachment 186225 [details] Patch
(In reply to comment #21) > Created an attachment (id=186225) [details] > Patch I made an investigation and found that we calculates the class names for 624 classes, instrumented and not instrumented. I reduced the signature of the function and moved the type independent code from the header to cpp. As a result the patch adds to the binary about 114k. We could reduce the footprint of the instrumentation but it should be a separate patch.
Created attachment 186227 [details] Patch
Comment on attachment 186227 [details] Patch Attachment 186227 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16340819
Comment on attachment 186227 [details] Patch Attachment 186227 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16342789 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Comment on attachment 186227 [details] Patch Attachment 186227 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16336828 New failing tests: inspector/profiler/memory-instrumentation-cached-images.html
Created attachment 186348 [details] Patch
Comment on attachment 186348 [details] Patch Attachment 186348 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16370196
Created attachment 186365 [details] with fix for windows bot
Comment on attachment 186365 [details] with fix for windows bot How much does this change binary size? The reason why we disabled RTTI was that it reduced our binary size by 1/3rd :)
(In reply to comment #30) > (From update of attachment 186365 [details]) > How much does this change binary size? The reason why we disabled RTTI was that it reduced our binary size by 1/3rd :) it is about 119.5MB at the moment. So with this patch it will increase by 0.09%
(In reply to comment #31) > (In reply to comment #30) > > (From update of attachment 186365 [details] [details]) > > How much does this change binary size? The reason why we disabled RTTI was that it reduced our binary size by 1/3rd :) > > it is about 119.5MB at the moment. So with this patch it will increase by 0.09% According to the build-bot data http://build.chromium.org/f/chromium/perf/xp-release/sizes/report.html?history=150&rev=-1&graph=chrome.dll the size on win32 is about 54MB. Yeah, I know this side effect of RTTI flag :) The alternative solution is to hardcode the class names in reportMemoryUsage and use <unknown> class name for the classes without instrumentation. Also template container class names will loose details like HashSet instead of HashSet<AnObject>.
Created attachment 186558 [details] with small fix for msvc version
Comment on attachment 186558 [details] with small fix for msvc version View in context: https://bugs.webkit.org/attachment.cgi?id=186558&action=review > Source/WTF/wtf/MemoryInstrumentation.h:-226 > - callReportObjectInfo(memoryObjectInfo, object, 0, sizeof(T)); callReportObjectInfo seems to be unused, can we remove it? > Source/WTF/wtf/MemoryInstrumentation.h:254 > + WTF_EXPORT_PRIVATE static void setInfo(MemoryObjectInfo*, const void* pointer, const char* stringWithClassName, MemoryObjectType, size_t actualSize); maybe callReportObjectInfo ?
Created attachment 186778 [details] comments addressed
Comment on attachment 186778 [details] comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=186778&action=review > Source/WTF/wtf/MemoryInstrumentation.cpp:-64 > - memoryObjectInfo->reportObjectInfo(pointer, objectType, objectSize); You need to remove it from header as well.
Created attachment 186785 [details] comments addressed
Committed r141992: <http://trac.webkit.org/changeset/141992>