Possible optimization to SkString

Possible optimization to SkString

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/976386b75d883eebaa4d2f19c4ae0222f3f85c0a

[joshualitt, 2014-10-23 22:45:47]

joshualitt@google.com changed reviewers:
+ joshualitt@google.com, mtklein@google.com

[joshualitt, 2014-10-23 22:45:47]

Mike, was there any reason you can think of why we were doing this unneeded
strlen in SkString?

[mtklein, 2014-10-23 23:01:36]

I don't think SkString has really ever had much performance scrutiny.  I agree
that this is certainly a good thing to do.  No point throwing away data.

I suspect we'll get even more win by entirely eliminating calls to vsnprintf, or
really anything ending with 'f'.  E.g. adding and using specialized methods like
append(SkScalar), append(int), etc.

https://codereview.chromium.org/679433003/diff/40001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/679433003/diff/40001/src/core/SkString.cpp#ne...
src/core/SkString.cpp:33: written = VSNPRINTF(buffer, size, format, args); \
Let's SkASSERT(written >= 0 && written < size) here?  The return value is more
like wouldHaveWrittenHadWeTheRoomToDoSo, so it's nice to check it's actually
less than size.

[joshualitt, 2014-10-23 23:13:46]

Good idea with the assert.

I like the idea of specialized append methods.  Actually, I think we should
switch over to a stream based model instead of using format strings at all. 
This would really help the readability of our shaders, and with proper design
could be just as faster or faster to using varargs.

https://codereview.chromium.org/679433003/diff/40001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/679433003/diff/40001/src/core/SkString.cpp#ne...
src/core/SkString.cpp:33: written = VSNPRINTF(buffer, size, format, args); \
On 2014/10/23 23:01:36, mtklein wrote:
> Let's SkASSERT(written >= 0 && written < size) here?  The return value is more
> like wouldHaveWrittenHadWeTheRoomToDoSo, so it's nice to check it's actually
> less than size.

Acknowledged.

[joshualitt, 2014-10-23 23:16:37]

https://codereview.chromium.org/679433003/diff/80001/src/core/SkString.cpp
File src/core/SkString.cpp (right):

https://codereview.chromium.org/679433003/diff/80001/src/core/SkString.cpp#ne...
src/core/SkString.cpp:563: SkASSERT(length >= 0);
whoops I can remove these, will update the patch

[mtklein, 2014-10-23 23:34:43]

lgtm

[joshualitt, 2014-10-24 01:13:23]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2014-10-24 01:14:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/679433003/100001

[commit-bot: I haz the power, 2014-10-24 01:23:42]

Committed patchset #6 (id:100001) as 976386b75d883eebaa4d2f19c4ae0222f3f85c0a