[Webkit-unassigned] [Bug 51311] SegmentedString should provide column position

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Adam Barth <abarth at webkit.org>  2010-12-22 00:40:28 PST ---
> > Can you run the benchmark with and without your patch so we can see how much performance we're talking about?  Per-character branches usually are visible on the benchmark.
> 
> I ran 3 times before and after, and in average I have:
> 4354 (base) and 4377 (with patch). It's definitely visible, but I'm not sure I see a really better solution.

Ok.  I think we can live with a 0.5% regression.

> > > 4. HTMLTreeBuilder uses m_parser new field, but it has to upcast in order to call textPosition. I don't see how to make it better.
> > 
> > Can't you just pass in the textPosition instead of passing in the whole object?
> 
> I'm sorry I don't quite see how I can pass it other way. Now I do a callback from tree builder as deep as to SegmentedString when I need a column value (at script tag only).
> 
> Passing it directly from parser as a parameter might be a bit messy. Here's a stack I would have to go through:
>         WebCore::HTMLTreeBuilder::processScriptStartTag()
>         WebCore::HTMLTreeBuilder::processStartTagForInHead()
>         WebCore::HTMLTreeBuilder::processStartTag()
>         WebCore::HTMLTreeBuilder::processToken()
>         WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken()
>         WebCore::HTMLTreeBuilder::constructTreeFromToken()
> 
> Alternatively I can create a variable somewhere, pass it by reference into the tree builder and update this reference on each token in parser. It seems to be quite fragile to me.
> 
> Do I miss something?

Ah, I think the change to HTMLTreeBuilder::processScriptStartTag isn't in your patch.  :)

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