RESOLVED FIXED Bug 96405
Web Inspector: NMI move String* instrumentation to wtf 
https://bugs.webkit.org/show_bug.cgi?id=96405
Summary Web Inspector: NMI move String* instrumentation to wtf
Ilya Tikhonovsky
Reported 2012-09-11 10:19:57 PDT
It is hard to upstream instrumentation traits for string classes. It is much simple to instrument string classes directly. Also this instrumentation is solving the problem with SubStrings.
Attachments
Patch (13.02 KB, patch)
2012-09-11 10:25 PDT, Ilya Tikhonovsky 		no flags
DetailsFormatted DiffDiff
Patch (17.09 KB, patch)
2012-09-12 01:46 PDT, Ilya Tikhonovsky 		yurys: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ilya Tikhonovsky
Comment 1 2012-09-11 10:25:18 PDT
Created attachment 163398 [details] Patch
Yury Semikhatsky
Comment 2 2012-09-12 01:34:40 PDT
Comment on attachment 163398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163398&action=review > Source/WTF/wtf/text/StringImpl.h:723 > + // count size used by internal buffer but skip strings that were constructed from literals. count -> Count > Source/WTF/wtf/text/StringImpl.h:731 > + typename MemoryObjectInfo::ClassInfo info(memoryObjectInfo, this, 0, selfSize); You should be consistent in using either constant MemoryObjectType::Unknown or 0 everywhere. > Source/WTF/wtf/text/StringImpl.h:735 > + else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substrings never have it's own shadow. typo: Substring never has its own shadow.
Ilya Tikhonovsky
Comment 3 2012-09-12 01:46:50 PDT
Created attachment 163546 [details] Patch
Yury Semikhatsky
Comment 4 2012-09-12 01:52:57 PDT
Comment on attachment 163546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163546&action=review > Source/WTF/wtf/text/StringImpl.h:735 > + else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substring never has it's own shadow. it's -> its
Ilya Tikhonovsky
Comment 5 2012-09-12 01:57:46 PDT
(In reply to comment #2) > (From update of attachment 163398 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163398&action=review > > > Source/WTF/wtf/text/StringImpl.h:723 > > + // count size used by internal buffer but skip strings that were constructed from literals. > > count -> Count Done > > > Source/WTF/wtf/text/StringImpl.h:731 > > + typename MemoryObjectInfo::ClassInfo info(memoryObjectInfo, this, 0, selfSize); > > You should be consistent in using either constant MemoryObjectType::Unknown or 0 everywhere. Ok I think we can use 0 everywhere otherwise I need to have a separate header file for generic memory types and cpp file for the value. > > > Source/WTF/wtf/text/StringImpl.h:735 > > + else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substrings never have it's own shadow. > > typo: Substring never has its own shadow. Done.
Ilya Tikhonovsky
Comment 6 2012-09-12 02:13:06 PDT
Committed r128279: <http://trac.webkit.org/changeset/128279>
WebKit Review Bot
Comment 7 2012-09-12 02:39:55 PDT
Re-opened since this is blocked by 96487
WebKit Review Bot
Comment 8 2012-09-12 05:26:38 PDT
Comment on attachment 163546 [details] Patch Attachment 163546 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13811919 New failing tests: MemoryInstrumentationTest.visitFirstMemberInNonVirtualClass MemoryInstrumentationTest.dataRef MemoryInstrumentationTest.sizeOf inspector/profiler/memory-instrumentation-cached-images.html MemoryInstrumentationTest.ptrVsRef MemoryInstrumentationTest.refPtr MemoryInstrumentationTest.ownerTypePropagation MemoryInstrumentationTest.ownPtr MemoryInstrumentationTest.ownPtrNotInstrumented MemoryInstrumentationTest.visitStrings
Ilya Tikhonovsky
Comment 9 2012-09-12 05:49:52 PDT
Committed r128300: <http://trac.webkit.org/changeset/128300>
Note You need to log in before you can comment on or make changes to this bug.