Shrink QueryTracker when freeing everything

Shrink QueryTracker when freeing everything

Before this, FreeEverything wouldn't free the shm associated with the
QueryTracker, even when all queries have been deleted. shm being at a 2MB
granularity, it leaves at least 2MB of shm per page. This fixes it.

It removes the bucketing of QuerySyncs. The rationale is that the
MappedMemoryManager already does pooling in shm chunks, and one extra level of
pooling seems redundant.

BUG=None


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

[greggman, 2013-02-16 02:09:28]

The reason there's a bucket is queries are rarely freed and having 8 byte
allocations hanging around the transfer buffer means more fragmentation and lots
more memory allocation.

[piman, 2013-02-16 02:19:49]

On 2013/02/16 02:09:28, greggman wrote:
> The reason there's a bucket is queries are rarely freed and having 8 byte
> allocations hanging around the transfer buffer means more fragmentation and
lots
> more memory allocation.

(they're 16 bytes).
https://codereview.chromium.org/12284013/ adds logic to free the queries when
the page goes to the background. Together with this it correctly frees the
shared memory, i.e. 2MB per tab.
At the same time we never have a lot of queries, they tend to be allocated
together, so I'm not too worried about fragmentation. Paying 2MB for that is
steep.
If you really prefer, I can put the buckets back and a query count or something
to track which bucket is in use. I'm just not sure the complexity is worth it.

[greggman, 2013-02-16 03:03:44]

On 2013/02/16 02:19:49, piman wrote:
> On 2013/02/16 02:09:28, greggman wrote:
> > The reason there's a bucket is queries are rarely freed and having 8 byte
> > allocations hanging around the transfer buffer means more fragmentation and
> lots
> > more memory allocation.
> 
> (they're 16 bytes).
> https://codereview.chromium.org/12284013/ adds logic to free the queries when
> the page goes to the background. Together with this it correctly frees the
> shared memory, i.e. 2MB per tab.
> At the same time we never have a lot of queries, they tend to be allocated
> together, so I'm not too worried about fragmentation. Paying 2MB for that is
> steep.
> If you really prefer, I can put the buckets back and a query count or
something
> to track which bucket is in use. I'm just not sure the complexity is worth it.

My point is this. Right now memory allocation looks like this

[QQQQQQ............................................]

Were Q is the bucket of queries. With this change it will look like this

[Q...Q.........Q...Q............Q..Q....Q.....Q....]

Which will most likely mean textures won't fit between the spaces
and we'll allocate another 2meg block (or 2 or 3) as IIRC the code allocates
queries on the fly.

Is that okay with you?

[piman, 2013-02-16 03:12:32]

On Fri, Feb 15, 2013 at 7:03 PM, <gman@chromium.org> wrote:

> On 2013/02/16 02:19:49, piman wrote:
>
>> On 2013/02/16 02:09:28, greggman wrote:
>> > The reason there's a bucket is queries are rarely freed and having 8
>> byte
>> > allocations hanging around the transfer buffer means more fragmentation
>> and
>> lots
>> > more memory allocation.
>>
>
>  (they're 16 bytes).
>>
https://codereview.chromium.**org/12284013/<https://codereview.chromium.org/1...
logic to free the queries when
>> the page goes to the background. Together with this it correctly frees the
>> shared memory, i.e. 2MB per tab.
>> At the same time we never have a lot of queries, they tend to be allocated
>> together, so I'm not too worried about fragmentation. Paying 2MB for that
>> is
>> steep.
>> If you really prefer, I can put the buckets back and a query count or
>>
> something
>
>> to track which bucket is in use. I'm just not sure the complexity is
>> worth it.
>>
>
> My point is this. Right now memory allocation looks like this
>
> [QQQQQQ.......................**.....................]
>
> Were Q is the bucket of queries. With this change it will look like this
>
> [Q...Q.........Q...Q..........**..Q..Q....Q.....Q....]
>
> Which will most likely mean textures won't fit between the spaces
> and we'll allocate another 2meg block (or 2 or 3) as IIRC the code
> allocates
> queries on the fly.
>
> Is that okay with you?
>

I guess my point was that as queries are reused, and they tend to be
allocated together, fragmentation would be minimal. Also we have relatively
few of them (at best about the number of tiles we will upload in a frame,
which is less than 100). That can only fragment so much. And *if* we're
unlucky and it fragments so much that it allocates an extra 2MB chunk on a
visible tab, it doesn't really bother me. What bothers me more is 2MB
wasted on every single background tab.
But I don't really want to spend the time arguing about whether we truly
fragment or not, I uploaded a new patch that restores the buckets. Up to
you...


>
>
>
>
https://codereview.chromium.**org/12278025/<https://codereview.chromium.org/1...
>

[greggman, 2013-02-16 05:14:43]

lgtm

[commit-bot: I haz the power, 2013-02-16 08:09:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/12278025/5002

[commit-bot: I haz the power, 2013-02-16 18:03:03]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-02-16 22:07:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/12278025/5002

[commit-bot: I haz the power, 2013-02-17 05:04:26]

Change committed as 183019