Bug 89889 – Web Inspector: replace recursion with a stack in DOM nodes snapshot traversal.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Alexei Filippov]

Number of DOM nodes native snapshots can handle is currently limited by the process stack size because of recursion used to traverse the nodes.
Change the recursion to a stack based algorithm.

[Alexei Filippov]

Created attachment 149315 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 149315 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149315&action=review

> Source/WebCore/dom/MemoryInstrumentation.h:40
> +class MemoryInstrumentedPointerBase;

Forward declarations should go in alphabetic order.

> Source/WebCore/dom/MemoryInstrumentation.h:111
> +    void countObjectSize()

Please inline this method instead.

> Source/WebCore/dom/MemoryInstrumentation.h:134
> +    void processInstrumentedPointer(MemoryInstrumentation* memoryInstrumentation)

missing virtual and OVERRIDE

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:451
> +            delete pointer;

Please use queue of OwnPtr's to avoid manual delete.

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:475
> +    Vector<MemoryInstrumentedPointerBase*> m_worklist;

m_instrumentedPointers?

[Alexei Filippov]

Created attachment 149536 [details]
Patch

[Alexei Filippov]

Comment on attachment 149315 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149315&action=review

>> Source/WebCore/dom/MemoryInstrumentation.h:40
>> +class MemoryInstrumentedPointerBase;
> 
> Forward declarations should go in alphabetic order.

done

>> Source/WebCore/dom/MemoryInstrumentation.h:111
>> +    void countObjectSize()
> 
> Please inline this method instead.

done

>> Source/WebCore/dom/MemoryInstrumentation.h:134
>> +    void processInstrumentedPointer(MemoryInstrumentation* memoryInstrumentation)
> 
> missing virtual and OVERRIDE

done

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:451
>> +            delete pointer;
> 
> Please use queue of OwnPtr's to avoid manual delete.

done

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:475
>> +    Vector<MemoryInstrumentedPointerBase*> m_worklist;
> 
> m_instrumentedPointers?

m_deferredInstrumentedPointers

[Ilya Tikhonovsky]

Comment on attachment 149536 [details]
Patch

lgtm

[Yury Semikhatsky]

Comment on attachment 149536 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149536&action=review

> Source/WebCore/dom/MemoryInstrumentation.h:120
> +class MemoryInstrumentedPointerBase {

I'd move these classes into MemoryInstrumentation as they are just implementation details. It would also allow us to give them shorter names.

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:476
> +    Vector<PassOwnPtr<MemoryInstrumentedPointerBase> > m_deferredInstrumentedPointers;

PassOwnPtr is supposed to be used for passing objects ownership only, you should use Vector<OwnPtr<...> > here.

[Alexei Filippov]

Created attachment 149720 [details]
Patch

[Alexei Filippov]

Created attachment 150405 [details]
Patch

[Alexei Filippov]

Comment on attachment 149536 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149536&action=review

>> Source/WebCore/dom/MemoryInstrumentation.h:120
>> +class MemoryInstrumentedPointerBase {
> 
> I'd move these classes into MemoryInstrumentation as they are just implementation details. It would also allow us to give them shorter names.

done

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:476
>> +    Vector<PassOwnPtr<MemoryInstrumentedPointerBase> > m_deferredInstrumentedPointers;
> 
> PassOwnPtr is supposed to be used for passing objects ownership only, you should use Vector<OwnPtr<...> > here.

done

[WebKit Review Bot]

Comment on attachment 150405 [details]
Patch

Clearing flags on attachment: 150405

Committed r121677: <http://trac.webkit.org/changeset/121677>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.