Bug 96405

Summary:

Web Inspector: NMI move String* instrumentation to wtf

Product:

WebKit

Reporter:

Ilya Tikhonovsky <loislo>

Component:

Web Inspector (Deprecated)

Assignee:

Ilya Tikhonovsky <loislo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alph, apavlov, benjamin, bweinstein, dglazkov, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

96487

Bug Blocks:

96367

Attachments:

Description	Flags
Patch	none
Patch	yurys: review+, webkit.review.bot: commit-queue-

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	Details Formatted Diff Diff
Patch (17.09 KB, patch) 2012-09-12 01:46 PDT, Ilya Tikhonovsky	yurys: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
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.