speedup peek32() when the offset is in the last block (fTail)

speedup peek32() when the offset is in the last block (fTail)
Committed: https://code.google.com/p/skia/source/detail?r=6708

[Leon, 2012-12-07 16:13:38]

Are there benchmarks that show improvement with this change?

https://codereview.appspot.com/6906047/diff/1/include/core/SkWriter32.h
File include/core/SkWriter32.h (right):

https://codereview.appspot.com/6906047/diff/1/include/core/SkWriter32.h#newco...
include/core/SkWriter32.h:201: uint32_t    fAllocatedBeforeLastBlock;
I think the name is not quite right. It's actually the amount used before the
last block, since it's possible that the second to last block (or any prior
block, really) has some extra space allocated that was skipped over.

https://codereview.appspot.com/6906047/diff/1/src/core/SkWriter32.cpp
File src/core/SkWriter32.cpp (right):

https://codereview.appspot.com/6906047/diff/1/src/core/SkWriter32.cpp#newcode218
src/core/SkWriter32.cpp:218: }
Should we also set fAllocatedBeforeLastBlock?

[reed1, 2012-12-07 16:54:03]

https://codereview.appspot.com/6906047/diff/1/include/core/SkWriter32.h
File include/core/SkWriter32.h (right):

https://codereview.appspot.com/6906047/diff/1/include/core/SkWriter32.h#newco...
include/core/SkWriter32.h:201: uint32_t    fAllocatedBeforeLastBlock;
On 2012/12/07 16:13:38, scroggo-work wrote:
> I think the name is not quite right. It's actually the amount used before the
> last block, since it's possible that the second to last block (or any prior
> block, really) has some extra space allocated that was skipped over.

Done.

https://codereview.appspot.com/6906047/diff/1/src/core/SkWriter32.cpp
File src/core/SkWriter32.cpp (right):

https://codereview.appspot.com/6906047/diff/1/src/core/SkWriter32.cpp#newcode218
src/core/SkWriter32.cpp:218: }
On 2012/12/07 16:13:38, scroggo-work wrote:
> Should we also set fAllocatedBeforeLastBlock?

Doh! Great catch.
Done.

[Leon, 2012-12-07 17:45:44]

LGTM

https://codereview.appspot.com/6906047/diff/6001/src/core/SkWriter32.cpp
File src/core/SkWriter32.cpp (right):

https://codereview.appspot.com/6906047/diff/6001/src/core/SkWriter32.cpp#newc...
src/core/SkWriter32.cpp:210: fWrittenBeforeLastBlock = originalOffset - offset;
Small nit. These names confused me when trying to understand these lines. Maybe
'requestedOffset' instead of originalOffset (which made me think the offset
before this function, though obviously the code tells me otherwise). I can't
think of a better name for 'offset' ('localOffset' sounds good at this point,
although it's not true for the entire use of the variable...).

Maybe a comment explaining what two numbers are being subtracted would help.