[Webkit-unassigned] [Bug 27511] Add WinCE specific platform/graphics files to WebCore

[bugzilla-daemon at webkit.org]

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





--- Comment #63 from Peter Kasting <pkasting at google.com>  2009-08-09 14:25:40 PDT ---
(In reply to comment #60)
> Heap allocation is slower than stack allocation. 2K is arbitrary.

I know heap allocation is slower :).  You didn't answer my question.  Does
WTF::Vector have any code in it to automatically stack-allocate for smaller
vectors?  (There are certainly Gecko classes that do...)

I would definitely comment in the code about 2k being arbitrary, and why you're
doing it.

> In the case that compiler doesn't optimize, *foo++ may result this kind of
> machine code 
> 1) move foo to register
> 2) save *foo to register or some variable
> 3) increase the register by one that keeps foo
> 4) save the register back to stack
> 
> But if ++foo is called later, then = *foo moves foo to register once, and later
> ++foo does that again.

For RISC chips that need to have something in a register to update it, the
compiler will allocate |foo| to a register for the body of the loop because
it's the loop index.  For chips (like x86, IIRC... x86 assembly is not my
strong suit) that can directly modify stack variables, no movement in and out
of a register is necessary.

The reason I suggest putting ++pixel in the for() declaration is that it's much
clearer to a reader what's happening in the loop -- you see the test and
iteration on one line, not broken into pieces and hidden in a later statement's
side effect.  From an optimization perspective, this kind of code helps the
compiler convert the loop into a decrementing loop, unroll it, etc.

> But the point is not that, I have no problem with choosing either one. I just
> think it may be a time waste on such small things.

There is no WebKit style rule on this one, but I don't consider clarity a small
thing, and I'll always suggest what I think is the clearest code.  Statements
with side effects are inherently less clear, even if (as in this case) the code
is simple enough that it's not too much more difficult to determine what the
code is doing.

Note that writing your loop iteration as suggested also makes it slightly
easier if we convert the code to use some kind of iterator or other interface
to get at the source data -- which we will likely need to do when we combine
this code with the other platforms'.  See also how the image decoders changed
to not using direct memory access in their inner loops, in favor of an inline
function in the header to do the right thing depending on the platform.

> In this case, I agree that while() is better. But what's your opinion about
> "while(true)" vs. "for(;;)"?

I have used both.  while(true) is significantly more common, and thus easier
for most people to read, and therefore slightly better IMO.

(In reply to comment #61)
> About "m_" for the members in struct: this is a good example that one reviewer
> says this, but another says opposite. Sigh...
> 
> I'm going to use:
> 
> class JpegDestinationManager
> {
> public:
>     Foo m_foo;
>     ...
> };
> 
> Will anyone say that the data member shouldn't be public? or suggest me to
> change back to struct?

I don't know what WebKit style is on that last question.  Google style (which I
only mention because it's the set of style rules I happen to know well) would
suggest you use a struct rather than a class for something that's purely a
container of public members, and personally I agree; I would leave it as a
struct and change the names as you suggested.  Someone like Eric should have
the last word on that kind of thing, though.

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