Remove unnecessary copies in CRLSet parsing.

Remove unnecessary copies in CRLSet parsing.

This showed up in Chrome Traces covering the startup of Chrome for Android.
CRLSet parsing was taking 70ms of CPU on Nexus 4 (although this is happening on
the File thread).
That cost is now down to 18ms. There are certainly further ways to decrease
this even more though.

BUG=376793
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274607

[Philippe, 2014-05-26 18:09:42]

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.h
File net/cert/crl_set.h (right):

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.h#newcode8
net/cert/crl_set.h:8: #include <map>
JFYI, I will remove this in the next patch set.

[pasko, 2014-05-26 18:41:11]

overall looks nice!

it would be interesting to know an estimate of a typical crl_set size, can you
ask agl@ about that?

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.cc
File net/cert/crl_set.cc (right):

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.cc#newcode169
net/cert/crl_set.cc:169: memcpy(&serial_length, data->data(), sizeof(uint8));
maybe:

serial_length = ((uint8*)data->data())[0]

?

[Philippe, 2014-05-26 18:53:39]

Thanks, Egor!

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.cc
File net/cert/crl_set.cc (right):

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.cc#newcode169
net/cert/crl_set.cc:169: memcpy(&serial_length, data->data(), sizeof(uint8));
On 2014/05/26 18:41:11, pasko wrote:
> maybe:
> 
> serial_length = ((uint8*)data->data())[0]
> 
> ?

Done.

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.h
File net/cert/crl_set.h (right):

https://codereview.chromium.org/302643003/diff/1/net/cert/crl_set.h#newcode8
net/cert/crl_set.h:8: #include <map>
On 2014/05/26 18:09:42, Philippe wrote:
> JFYI, I will remove this in the next patch set.

Done.

[Philippe, 2014-05-28 11:26:29]

Adding Adam.

[Philippe, 2014-06-02 11:47:10]

On 2014/05/28 11:26:29, Philippe wrote:
> Adding Adam.

Ping Adam :)

[agl, 2014-06-02 18:07:25]

LGTM

https://codereview.chromium.org/302643003/diff/20001/net/cert/crl_set.cc
File net/cert/crl_set.cc (right):

https://codereview.chromium.org/302643003/diff/20001/net/cert/crl_set.cc#newc...
net/cert/crl_set.cc:162: out_serials->reserve(num_serials);
Previously the memory usage of this code was limited by the size of the file.
Now, a corruption can kill Chromium at startup.

Can you sanity check num_serials < 32 * 1024 * 1024 and fail to parse otherwise?

[Philippe, 2014-06-03 08:21:03]

Thanks, Adam!

https://codereview.chromium.org/302643003/diff/20001/net/cert/crl_set.cc
File net/cert/crl_set.cc (right):

https://codereview.chromium.org/302643003/diff/20001/net/cert/crl_set.cc#newc...
net/cert/crl_set.cc:162: out_serials->reserve(num_serials);
On 2014/06/02 18:07:25, agl wrote:
> Previously the memory usage of this code was limited by the size of the file.
> Now, a corruption can kill Chromium at startup.
> 
> Can you sanity check num_serials < 32 * 1024 * 1024 and fail to parse
otherwise?

Good point :)

[Philippe, 2014-06-03 08:21:16]

The CQ bit was checked by pliard@chromium.org

[commit-bot: I haz the power, 2014-06-03 08:21:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pliard@chromium.org/302643003/40001

[Philippe, 2014-06-03 11:09:53]

The CQ bit was unchecked by pliard@chromium.org

[Philippe, 2014-06-03 11:17:31]

The CQ bit was checked by pliard@chromium.org

[commit-bot: I haz the power, 2014-06-03 11:18:30]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pliard@chromium.org/302643003/60001

[commit-bot: I haz the power, 2014-06-03 17:17:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-03 20:34:07]

Change committed as 274607