[Webkit-unassigned] [Bug 30707] WebInspector: Refactors InspectorTimelineAgent to eliminate TimelineItem classes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30707





--- Comment #4 from Kelly Norton <knorton at google.com>  2009-10-23 10:10:44 PDT ---
A Vector will work just fine, here's the reason I used a singly linked list:

Without getters on ScriptObject's there is no getting around having a C++
wrapper object because you need a place to keep the m_children ScriptArray.
Since you have to create a C++ object anyway, it would be better to have
constant-time push/pop of a linked list rather than the aggregated
constant-time of a sensibly growing Vector (I haven't even looked to see if
WTF::Vector uses a growth strategy that will give us aggregated constant time).

So, a Vector will work just fine here but it's actually too complex a data
structure for our needs and adds some unfortunate runtime complexity. But I
think it's a pretty minor detail overall, so I'll defer to your judgement.


(In reply to comment #2)
> (From update of attachment 41715 [details])
> I see how lack of getters hurts you, sorry about that - we should probably
> implement those. It would be great if you made the stack explicit other than
> encapuslating it into your new class. It would look really short then, no heap
> allocations, brought back assert. What do I miss, why the additional
> complexity?
> 
> struct TimelineEventEntry {
>     public TimelineEvent(object, children, type) : object(object),
> children(children), type(type) {}
>     ScriptObject object;
>     ScriptArray children;
>     int type;
> }
> 
> Vector< TimelineEventEntry > m_eventStack;
> 
> 
> InspectorTimelineAgent::willRecalculateStyle()
> {
>     pushTimelineItem(TimelineItemFactory::createGenericTimelineItem(m_frontend,
> currentTimeInMilliseconds()), RecalculateStylesTimelineItemType);
> }
> 
> InspectorTimelineAgent::didRecalculateStyle()
> {
>     popTimelineItem(RecalculateStylesTimelineItemType);
> }
> 
> InspectorTimelineAgent::pushTimelineItem(ScriptObject event, int type)
> {
>     ScriptArray children = m_frontend->newScriptArray();
>     event.set("type", type);
>     event.set("children", children);
>     m_eventStack.append(TimelineEventEntry(event, children, type));
> }
> 
> InspectorTimelineAgent::popTimelineItem(int type)
> {
>     TimelineEventEntry event = m_eventStack.last();
>     m_eventStack.removeLast();
>     ASSERT(event.type == type);
>     event.object.set("endTime", currentTimeInMilliseconds());
> 
>     if (m_eventStack.size()) {
>         TimelineEventEntry parent = m_eventStack.last();
>         parent.children.set(parent.children.length(), event.object);
>     } else {
>         m_frontend->addItemToTimeline(event.object);
>     }
> }

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.