SkPDF: alloc less memory for strings

SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

Also, be slightly more space-efficient in encoding strings.

Also, add a bench.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002

Committed: https://skia.googlesource.com/skia/+/ee41b7556dfab48509d61854d256ee523cb8ee4c

[hal.canary, 2016-06-23 19:59:48]

Description was changed from

==========
SkPDF:  alloc less memory for strings

BUG=skia:
==========

to

==========
SkPDF:  alloc less memory for strings

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

[hal.canary, 2016-06-23 20:04:18]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 20:04:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2099463002/1

[commit-bot: I haz the power, 2016-06-23 20:10:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 20:10:14]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[hal.canary, 2016-06-23 20:16:49]

Patchset #2 (id:20001) has been deleted

[hal.canary, 2016-06-23 20:23:44]

Description was changed from

==========
SkPDF:  alloc less memory for strings

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

to

==========
SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

[hal.canary, 2016-06-23 20:23:56]

Patchset #1 (id:1) has been deleted

[hal.canary, 2016-06-23 20:24:05]

Patchset #1 (id:40001) has been deleted

[hal.canary, 2016-06-23 20:28:10]

Description was changed from

==========
SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

to

==========
SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

Also, be slightly more space-efficient in encoding strings.

Also, add a bench.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

[hal.canary, 2016-06-23 20:29:17]

The CQ bit was checked by halcanary@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-23 20:29:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2099463002/60001

[hal.canary, 2016-06-23 20:31:02]

halcanary@google.com changed reviewers:
+ tomhudson@google.com

[hal.canary, 2016-06-23 20:31:02]

PTAL

[reed1, 2016-06-23 20:37:19]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2016-06-23 20:37:20]

The bench is speed? but the CL talks about storage?

What is the effect of the change, and how does it measure on non-bench content?

[tomhudson, 2016-06-23 20:39:11]

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp
File bench/PDFBench.cpp (right):

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp#newc...
bench/PDFBench.cpp:212: std::unique_ptr<SkWStream> fWStream;
We prefer std::unique_ptr<> to sk_sp<>, I suppose?

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp#newc...
bench/PDFBench.cpp:224: SkPDFUtils::WriteString(fWStream.get(), kBinary,
strlen(kBinary));
Are we getting a free invisible string terminator in kBinary?

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1089: SkAutoMalloc buffer(len);  // Remove every other
byte.
Seems like you haven't gotten rid of the allocation in this case, although I
suppose this wasn't what you were trying to optimize.

[tomhudson, 2016-06-23 20:43:45]

On 2016/06/23 20:37:20, reed1 wrote:
> The bench is speed? but the CL talks about storage?
> 
> What is the effect of the change, and how does it measure on non-bench
content?

Hal was expecting speedup because we aren't malloc()ing a string to hold
intermediate values on every write.

[commit-bot: I haz the power, 2016-06-23 20:51:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 20:51:39]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2016-06-23 20:54:06]

On 2016/06/23 20:37:20, reed1 wrote:
> The bench is speed? but the CL talks about storage?
> 
> What is the effect of the change, and how does it measure on non-bench
content?

The speedup is due to calling malloc one less time every time a string is
written.

On non-bench content, it is very hard to see the difference.

[hal.canary, 2016-06-23 20:57:12]

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp
File bench/PDFBench.cpp (right):

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp#newc...
bench/PDFBench.cpp:212: std::unique_ptr<SkWStream> fWStream;
On 2016/06/23 20:39:11, tomhudson wrote:
> We prefer std::unique_ptr<> to sk_sp<>, I suppose?

streams aren't ref-counted.

https://codereview.chromium.org/2099463002/diff/60001/bench/PDFBench.cpp#newc...
bench/PDFBench.cpp:224: SkPDFUtils::WriteString(fWStream.get(), kBinary,
strlen(kBinary));
On 2016/06/23 20:39:11, tomhudson wrote:
> Are we getting a free invisible string terminator in kBinary?

yes

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1089: SkAutoMalloc buffer(len);  // Remove every other
byte.
On 2016/06/23 20:39:11, tomhudson wrote:
> Seems like you haven't gotten rid of the allocation in this case, although I
> suppose this wasn't what you were trying to optimize.

Before, I had TWO allocations.  Can you see an easy way to get rid of this one,
other than adding complexity to SkPDFUtils::WriteString?

[tomhudson, 2016-06-23 21:03:51]

LGTM

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (right):

https://codereview.chromium.org/2099463002/diff/60001/src/pdf/SkPDFDevice.cpp...
src/pdf/SkPDFDevice.cpp:1089: SkAutoMalloc buffer(len);  // Remove every other
byte.
On 2016/06/23 20:57:12, Hal Canary wrote:
> On 2016/06/23 20:39:11, tomhudson wrote:
> > Seems like you haven't gotten rid of the allocation in this case, although I
> > suppose this wasn't what you were trying to optimize.
> 
> Before, I had TWO allocations.  Can you see an easy way to get rid of this
one,
> other than adding complexity to SkPDFUtils::WriteString?

The wideChars section avoids it, but I'd suggest we don't bother with that until
this becomes a bottleneck.

[hal.canary, 2016-06-23 21:07:16]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2016-06-23 21:07:22]

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

[commit-bot: I haz the power, 2016-06-23 21:08:13]

Description was changed from

==========
SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

Also, be slightly more space-efficient in encoding strings.

Also, add a bench.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002
==========

to

==========
SkPDF:  alloc less memory for strings

    before:
        micros   	bench
        250.98  	WritePDFText	nonrendering
    after:
        micros   	bench
        107.10  	WritePDFText	nonrendering

Also, be slightly more space-efficient in encoding strings.

Also, add a bench.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2099463002

Committed:
https://skia.googlesource.com/skia/+/ee41b7556dfab48509d61854d256ee523cb8ee4c
==========

[commit-bot: I haz the power, 2016-06-23 21:08:14]

Committed patchset #1 (id:60001) as
https://skia.googlesource.com/skia/+/ee41b7556dfab48509d61854d256ee523cb8ee4c