PDF: Use a vector instead of a list in DocumentLoader.

PDF: Use a vector instead of a list in DocumentLoader.

Committed: https://crrev.com/d9b9c9ec0097c4b544872ffa0ee03055aae7d859
Cr-Commit-Position: refs/heads/master@{#369803}

[Lei Zhang, 2016-01-15 07:02:14]

thestig@chromium.org changed reviewers:
+ spelchat@chromium.org

[spelchat, 2016-01-15 16:54:21]

On 2016/01/15 07:02:14, Lei Zhang wrote:

I'm assuming this helps because the list fragments memory too much?

The PDF is ~1.6 GB. At ~65 KB per chunk, that means we have ~25000 elements in
the list. So the overhead of using a list vs a vector (assuming 16 bytes
overhead per element, which I think is the case for libstdc++) is ~400 KB, which
I doubt makes a difference here.

That said the individual vectors are still not contiguous with those changes, so
I'm surprised memory fragmentation would be much better.

Am I missing something?

LGTM anyway, I'm just surprised it makes such a huge difference.

[spelchat, 2016-01-15 17:25:21]

On 2016/01/15 16:54:21, spelchat wrote:
> On 2016/01/15 07:02:14, Lei Zhang wrote:
> 
> I'm assuming this helps because the list fragments memory too much?
> 
> The PDF is ~1.6 GB. At ~65 KB per chunk, that means we have ~25000 elements in
> the list. So the overhead of using a list vs a vector (assuming 16 bytes
> overhead per element, which I think is the case for libstdc++) is ~400 KB,
which
> I doubt makes a difference here.
> 
> That said the individual vectors are still not contiguous with those changes,
so
> I'm surprised memory fragmentation would be much better.
> 
> Am I missing something?
> 
> LGTM anyway, I'm just surprised it makes such a huge difference.

For some reason I thought this was the CL linked from crbug.com/577981.
Nevermind my comment ;).

[Lei Zhang, 2016-01-15 18:08:21]

I just didn't see a reason to use a list considering all we do is append to the
end a bunch of times, and then iterate through it.

[Lei Zhang, 2016-01-15 18:08:25]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-01-15 18:09:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1587083004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1587083004/1

[commit-bot: I haz the power, 2016-01-15 19:18:28]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-15 19:19:10]

Description was changed from

==========
PDF: Use a vector instead of a list in DocumentLoader.
==========

to

==========
PDF: Use a vector instead of a list in DocumentLoader.

Committed: https://crrev.com/d9b9c9ec0097c4b544872ffa0ee03055aae7d859
Cr-Commit-Position: refs/heads/master@{#369803}
==========

[commit-bot: I haz the power, 2016-01-15 19:19:10]

Patchset 1 (id:??) landed as
https://crrev.com/d9b9c9ec0097c4b544872ffa0ee03055aae7d859
Cr-Commit-Position: refs/heads/master@{#369803}