Re: [webkit-dev] review queue crazy idea

2010-07-24 Thread Ryosuke Niwa
On Sat, Jul 24, 2010 at 4:09 PM, Maciej Stachowiak  wrote:

> I think the main problems with 
> are that (a) people don't know to look there; and (b) people don't know or
> don't bother to update it.
>

I totally agree.

I'll also add that the information on WebKit Team is too broad.  For
example, one can say that he's an expert in HTML editing but he might not be
too familiar with how TextIterator advances or how ApplyStyleCommand pushes
down text decorations.  I'm sure there are similar problems with other
subsystems as well.  Aggregating information out of the svn blame will solve
this problem as well.  I'd also recommend to provide a way to list
committers who recently touched that code as well because they are also
likely to be able to find problems with new patches.

Best,
Ryosuke Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] review queue crazy idea

2010-07-24 Thread Maciej Stachowiak

On Jul 23, 2010, at 1:10 PM, Dirk Pranke wrote:

> I have been thinking along these lines as well. I'm not sure how
> relevant touching existing lines of code is versus just other people
> who have hacked on the file at all or who have hacked on other files
> in the same directory (i.e., you'd need to address new code and new
> files, too). I think some empirical testing and tweaking would be the
> way to evaluate this, though.

To find a reviewer, what you want to find is people who understand the relevant 
subsystems well, and perhaps also people who are good reviewers in general 
(have a high likelihood of spotting avoidable problems). Making lots of commits 
to (or touching lots of lines in) the same file or the same directory are at 
best proxies for that kind of information. They may be better proxies for that 
kind of information than self-identification, but that has yet to be 
demonstrated. While an algorithm is a good starting point, I think 
self-identification and/or peer identification can capture nuances that an 
algorithm will not.

I think the main problems with  are 
that (a) people don't know to look there; and (b) people don't know or don't 
bother to update it. I don't think the accuracy of the information is a problem 
(other than possibly being out of date). I don't see how it is helpful to refer 
to this information as "bragging".

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] A Parallel Webkit?

2010-07-24 Thread Leo Meyerovich
I don't think that question is very pertinent to the list; it's more  
of a longterm thing. The es committee has little interest in it for  
the relevant future and I agree with this given the other needs,  
available slack time, and induced complexity. Luckily, a somewhat  
parallel browser does not need it, as I've hopefully made clear.



On Jul 24, 2010, at 1:51 PM, Oliver Hunt  wrote:


Shared-state parallel scripting (and concurrent DOM) seem inevitable
I don't know why you believe this -- there is no intention of having  
shared state threading in ES, nor is there any desire in the ES  
technical committee to add any for of concurrency primitives to the  
language.


--Oliver



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] A Parallel Webkit?

2010-07-24 Thread Oliver Hunt
> Shared-state parallel scripting (and concurrent DOM) seem inevitable
I don't know why you believe this -- there is no intention of having shared 
state threading in ES, nor is there any desire in the ES technical committee to 
add any for of concurrency primitives to the language.

--Oliver


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Crash in RenderLayer related to setStyle() - Questions

2010-07-24 Thread Dimitri Glazkov
Coincidentally, http://webkit.org/coding/dom-element-attach.html

:DG<

On Sat, Jul 24, 2010 at 9:53 AM, Dan Bernstein  wrote:
>
> On Jul 23, 2010, at 3:48 PM, Alex Milowski wrote:
>
>> I've identified a crash with the MathML implementation related to use of
>> CSS style rules that cause a RenderLayer instance to be created.  In the
>> MathML code's various createRenderer() methods, they always call
>> RenderObject::setStyle() on the object they've just created.
>>
>> When the setStyle() method is called, a style difference is calculated
>> and propagated.  If you call setStyle() inside your createRenderer()
>> method, you're calling it on an unattached RenderObject instance.
>>
>> Further, if there happens to be a rule like:
>>
>> math {
>>   overflow: auto;
>> }
>>
>> you'll get a layer created by RenderBoxModelObject.  That layer
>> will get the style change.
>>
>> Right now, as the code stands, you'll get an exception and crash
>> due to the fact that the RenderLayer code makes some assumptions
>> about the RenderObject instance being attached to the tree.
>>
>> The fix is trivial but my question is basically: what's the right thing
>> to do here?  Should the setStyle() be called in createRenderer() ?  It
>> seems like the answer is no because the style gets associated
>> somewhere else.  If I remove the call, the crash is also fixed (without
>> any change to RenderLayer) and the tests still all pass for MathML.
>>
>> Further, should RenderLayer be made more safe?  Should it check
>> for zero pointer values in a couple places and avoid this?
>>
>> If we shouldn't be calling setStyle() in createRenderer(), then fixing
>> RenderLayer would just hide that fact.  While an ASSERT wouldn't
>> hide things, it would still only arise when a layer is created.
>>
>> I know how to fix the MathML code and I just want to make
>> sure I understand why calling setStyle() is "bad".
>>
>> I'm not sure what should be done about RenderLayer or otherwise.
>>
>> Suggestions?
>
> Do not call setStyle() from createRenderer(). Node::createRendererIfNeeded() 
> calls setAnimatableStyle() on the renderer after setting it as the node’s 
> renderer, so there is no need for createRenderer() to call setStyle(); and as 
> you saw, calling it before setting the node’s renderer violates the 
> assumption that the pointers between node and renderer are in a consistent 
> state (which is not to say that renderer->node->renderer is always the 
> original renderer).
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Crash in RenderLayer related to setStyle() - Questions

2010-07-24 Thread Dan Bernstein

On Jul 23, 2010, at 3:48 PM, Alex Milowski wrote:

> I've identified a crash with the MathML implementation related to use of
> CSS style rules that cause a RenderLayer instance to be created.  In the
> MathML code's various createRenderer() methods, they always call
> RenderObject::setStyle() on the object they've just created.
> 
> When the setStyle() method is called, a style difference is calculated
> and propagated.  If you call setStyle() inside your createRenderer()
> method, you're calling it on an unattached RenderObject instance.
> 
> Further, if there happens to be a rule like:
> 
> math {
>   overflow: auto;
> }
> 
> you'll get a layer created by RenderBoxModelObject.  That layer
> will get the style change.
> 
> Right now, as the code stands, you'll get an exception and crash
> due to the fact that the RenderLayer code makes some assumptions
> about the RenderObject instance being attached to the tree.
> 
> The fix is trivial but my question is basically: what's the right thing
> to do here?  Should the setStyle() be called in createRenderer() ?  It
> seems like the answer is no because the style gets associated
> somewhere else.  If I remove the call, the crash is also fixed (without
> any change to RenderLayer) and the tests still all pass for MathML.
> 
> Further, should RenderLayer be made more safe?  Should it check
> for zero pointer values in a couple places and avoid this?
> 
> If we shouldn't be calling setStyle() in createRenderer(), then fixing
> RenderLayer would just hide that fact.  While an ASSERT wouldn't
> hide things, it would still only arise when a layer is created.
> 
> I know how to fix the MathML code and I just want to make
> sure I understand why calling setStyle() is "bad".
> 
> I'm not sure what should be done about RenderLayer or otherwise.
> 
> Suggestions?

Do not call setStyle() from createRenderer(). Node::createRendererIfNeeded() 
calls setAnimatableStyle() on the renderer after setting it as the node’s 
renderer, so there is no need for createRenderer() to call setStyle(); and as 
you saw, calling it before setting the node’s renderer violates the assumption 
that the pointers between node and renderer are in a consistent state (which is 
not to say that renderer->node->renderer is always the original renderer).
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] A Parallel Webkit?

2010-07-24 Thread Leo Meyerovich

>> JS compilation is also done lazily in webkit so we don't ever end up with 
>> multiple pieces of code to compile concurrently.


Two of us have been focusing on speeding up bottlenecks like these to allow 
such synchronous interfaces. However, frameworks like TBB push towards having 
parallel tasks that are at least 10-100,000 cycles for a good reason; working 
at such small scales is for people who have the luxury of targeting farther 
ahead and I wouldn't suggest doing it without help.



>> JS execution and DOM manipulation are single threaded, and that thread is 
>> the UI thread, this fact cannot be changed.  
> 
> We could potentially do HTML parsing on off the main thread.  Firefox
> 4 has support for this.  It's unclear how much of a win this would be.


What Adam is suggesting applies not only to HTML parsing and has seemed to me 
to be the pragmatic and likely path for the short-term (next 1-2 years). E.g, 
create buffers (hidden by synchronous interfaces where possible) -- for 
example, queueing up calls from the parser to the selector matcher -- and then 
handle those in bulk. In other places, like Adam's example of a call to the 
parser itself, it's harder, but still possible (while being  clean!) by 
approaches like passing around forceable futures.  In a sense, such an approach 
is already the norm in terms of the interface between JS and the DOM/layout (at 
least in Firefox): calls to layout etc. are buffered up to avoid repeated or 
unnecessary evaluation. While I don't think this approach will expose 
significant parallelism, I think it would match the available hardware -- if 
anyone is interested in doing such a limit study, we should talk!

Shared-state parallel scripting (and concurrent DOM) seem inevitable, but these 
sorts of things are the minority in the performance profile. E.g., when loading 
a page, about half the JavaScript time is spent in non-execution tasks like 
parsing and code generating. They're still a concern in the sense of Amdahl's 
law and enabling new types of applications, but for getting the web to work 
satisfyingly on mobile, there seem to be bigger blockers. Until research 
catches up with the algorithms in browsers, exposing and exploiting 
coarse-level concurrency between components would be my recommendation (outside 
of some more obvious algorithms).

Regards,

- Leo



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev