Run HostDiscardableSharedMemoryManager::AllocateLockedDiscardableSharedMemoryForChild on FILE_USER_…

Run HostDiscardableSharedMemoryManager::AllocateLockedDiscardableSharedMemoryForChild on FILE_USER_BLOCKING thread

Should prevent some jank on OS X (and Linux), as allocating shared memory is a filesystem operation there.

BUG=467781, 466405
Committed: https://crrev.com/4ae512e37d550a92108e5e939ed9dfa546358cbd
Cr-Commit-Position: refs/heads/master@{#321279}

[jbauman, 2015-03-18 00:50:33]

jbauman@chromium.org changed reviewers:
+ avi@chromium.org, reveman@chromium.org

[jbauman, 2015-03-18 00:50:33]

reveman@, please review

avi@: content/ OWNERS review.

[reveman, 2015-03-18 01:08:09]

Should we do the same for OnDeletedDiscardableSharedMemory? If not for
performance reasons, maybe for consistency.

[jbauman, 2015-03-18 01:11:09]

On 2015/03/18 01:08:09, reveman wrote:
> Should we do the same for OnDeletedDiscardableSharedMemory? If not for
> performance reasons, maybe for consistency.

I believe the deletion is a lot cheaper than the creation (the memory was
already unlinked from the filesystem by that point), and doing an extra thread
hop seems like a waste, so I'm inclined not.

[reveman, 2015-03-18 01:37:43]

On 2015/03/18 at 01:11:09, jbauman wrote:
> On 2015/03/18 01:08:09, reveman wrote:
> > Should we do the same for OnDeletedDiscardableSharedMemory? If not for
> > performance reasons, maybe for consistency.
> 
> I believe the deletion is a lot cheaper than the creation (the memory was
already unlinked from the filesystem by that point), and doing an extra thread
hop seems like a waste, so I'm inclined not.

I think OnDeletedDiscardableSharedMemory is actually affected by the performance
of OnAllocate because DeletedDiscardableSharedMemory need to acquire the lock
that could be held by AllocateLockedDiscardableSharedMemoryForChild.

[Avi (use Gerrit), 2015-03-18 02:19:29]

LGTM

[reveman, 2015-03-18 03:07:29]

reveman@chromium.org changed reviewers:
+ erikchen@chromium.org

[reveman, 2015-03-18 03:07:29]

+erikchen

Can we remove the profiling added for crbug.com/466405 and close that bug after
landing this? As part of this patch or as a follow up, either way is fine with
me.

[erikchen, 2015-03-18 03:12:27]

On 2015/03/18 03:07:29, reveman wrote:
> +erikchen
> 
> Can we remove the profiling added for crbug.com/466405 and close that bug
after
> landing this? As part of this patch or as a follow up, either way is fine with
> me.

Let's confirm that this task stops janking on the IO thread (since the heavy
lifting should now be on a different thread), and then removing the profiling.

[reveman, 2015-03-18 03:44:03]

On 2015/03/18 at 03:12:27, erikchen wrote:
> On 2015/03/18 03:07:29, reveman wrote:
> > +erikchen
> > 
> > Can we remove the profiling added for crbug.com/466405 and close that bug
after
> > landing this? As part of this patch or as a follow up, either way is fine
with
> > me.
> 
> Let's confirm that this task stops janking on the IO thread (since the heavy
lifting should now be on a different thread), and then removing the profiling.

Ok, sgtm.

[jbauman, 2015-03-18 23:10:09]

On 2015/03/18 01:37:43, reveman wrote:
> On 2015/03/18 at 01:11:09, jbauman wrote:
> > On 2015/03/18 01:08:09, reveman wrote:
> > > Should we do the same for OnDeletedDiscardableSharedMemory? If not for
> > > performance reasons, maybe for consistency.
> > 
> > I believe the deletion is a lot cheaper than the creation (the memory was
> already unlinked from the filesystem by that point), and doing an extra thread
> hop seems like a waste, so I'm inclined not.
> 
> I think OnDeletedDiscardableSharedMemory is actually affected by the
performance
> of OnAllocate because DeletedDiscardableSharedMemory need to acquire the lock
> that could be held by AllocateLockedDiscardableSharedMemoryForChild.

Fair enough. Fixed.

[reveman, 2015-03-19 02:23:12]

lgtm

[jbauman, 2015-03-19 02:51:15]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2015-03-19 02:51:15]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1007323005/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-03-19 02:51:27]

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

[commit-bot: I haz the power, 2015-03-19 02:55:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-19 02:56:00]

Patchset 3 (id:??) landed as
https://crrev.com/4ae512e37d550a92108e5e939ed9dfa546358cbd
Cr-Commit-Position: refs/heads/master@{#321279}