Revert of Move gpu memory calculations to Compositor.

Revert of Move gpu memory calculations to Compositor. (patchset #11 id:220001 of https://codereview.chromium.org/1324413003/ )

Reason for revert:
Hangs on Chrome OS when rotating the display.

BUG=546653
Original issue's description:
> Move gpu memory calculations to Compositor.
> 
> This moves gpu memory policy code from gpumemorymanager to renderer compositor.
> It will be useful for renderer to use off screen context.
> 
> BUG=526196
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/127a06b45820e30f78079814557cfacd038d6167
> Cr-Commit-Position: refs/heads/master@{#352301}

TBR=dcheng@chromium.org,boliu@chromium.org,ccameron@chromium.org,danakj@chromium.org,dtrainor@chromium.org,msw@chromium.org,piman@chromium.org,sky@chromium.org,sohan.jyoti@samsung.com
BUG=526196
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

[spang, 2015-10-29 17:21:37]

Created Revert of Move gpu memory calculations to Compositor.

[commit-bot: I haz the power, 2015-10-29 17:22:59]

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

[spang, 2015-10-29 17:24:46]

The CQ bit was unchecked by spang@chromium.org

[commit-bot: I haz the power, 2015-10-29 17:24:55]

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

[commit-bot: I haz the power, 2015-10-29 17:24:56]

Failed to apply patch for cc/trees/layer_tree_host_impl.cc:
While running git apply --index -3 -p1;
  error: patch failed: cc/trees/layer_tree_host_impl.cc:177
  Falling back to three-way merge...
  Applied patch to 'cc/trees/layer_tree_host_impl.cc' with conflicts.
  U cc/trees/layer_tree_host_impl.cc

Patch:       cc/trees/layer_tree_host_impl.cc
Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index
807ec03f9c978686b2d9ce983587ceef36e4ced4..ab75b9c0c1139833228d89ec68da995fc9ab0077
100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -129,6 +129,16 @@
   TRACE_EVENT_ASYNC_END0("cc", "LayerTreeHostImpl::SetVisible", id);
 }
 
+size_t GetDefaultMemoryAllocationLimit() {
+  // TODO(ccameron): (http://crbug.com/137094) This 64MB default is a straggler
+  // from the old texture manager and is just to give us a default memory
+  // allocation before we get a callback from the GPU memory manager. We
+  // should probaby either:
+  // - wait for the callback before rendering anything instead
+  // - push this into the GPU memory manager somehow.
+  return 64 * 1024 * 1024;
+}
+
 }  // namespace
 
 LayerTreeHostImpl::FrameData::FrameData()
@@ -177,7 +187,10 @@
       tile_priorities_dirty_(false),
       settings_(settings),
       visible_(true),
-      cached_managed_memory_policy_(settings.memory_policy_),
+      cached_managed_memory_policy_(
+          GetDefaultMemoryAllocationLimit(),
+          gpu::MemoryAllocation::CUTOFF_ALLOW_EVERYTHING,
+          ManagedMemoryPolicy::kDefaultNumResourcesLimit),
       is_synchronous_single_threaded_(!proxy->HasImplThread() &&
                                       !settings.single_thread_proxy_scheduler),
       // Must be initialized after is_synchronous_single_threaded_ and proxy_.

[spang, 2015-10-29 17:24:59]

Description was changed from

==========
Revert of Move gpu memory calculations to Compositor. (patchset #11 id:220001 of
https://codereview.chromium.org/1324413003/ )

Reason for revert:
Hangs on Chrome OS when rotating the display.

BUG=546653

Original issue's description:
> Move gpu memory calculations to Compositor.
> 
> This moves gpu memory policy code from gpumemorymanager to renderer
compositor.
> It will be useful for renderer to use off screen context.
> 
> BUG=526196
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/127a06b45820e30f78079814557cfacd038d6167
> Cr-Commit-Position: refs/heads/master@{#352301}

TBR=dcheng@chromium.org,boliu@chromium.org,ccameron@chromium.org,danakj@chrom...
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=526196
==========

to

==========
Revert of Move gpu memory calculations to Compositor. (patchset #11 id:220001 of
https://codereview.chromium.org/1324413003/ )

Reason for revert:
Hangs on Chrome OS when rotating the display.

BUG=546653

Original issue's description:
> Move gpu memory calculations to Compositor.
> 
> This moves gpu memory policy code from gpumemorymanager to renderer
compositor.
> It will be useful for renderer to use off screen context.
> 
> BUG=526196
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/127a06b45820e30f78079814557cfacd038d6167
> Cr-Commit-Position: refs/heads/master@{#352301}

TBR=dcheng@chromium.org,boliu@chromium.org,ccameron@chromium.org,danakj@chrom...
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=526196
==========

[commit-bot: I haz the power, 2015-10-29 17:26:23]

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

[spang, 2015-10-29 17:35:20]

The CQ bit was checked by spang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 17:35:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412923004/290001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412923004/290001

[spang, 2015-10-29 17:36:09]

The CQ bit was unchecked by spang@chromium.org

[spang, 2015-10-29 17:36:13]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-10-29 17:37:52]

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

[boliu, 2015-10-29 17:49:51]

Rebase not completely correct here, since I removed some files that was touched.
I can sort through it after though.

Although this is a huge patch being reverted after 3+ weeks. Pretty dangerous by
itself even if it was clean..

[danakj, 2015-10-29 17:51:16]

On Thu, Oct 29, 2015 at 10:49 AM, <boliu@chromium.org> wrote:

> Rebase not completely correct here, since I removed some files that was
> touched.
> I can sort through it after though.
>
> Although this is a huge patch being reverted after 3+ weeks. Pretty
> dangerous by
> itself even if it was clean..
>

I don't think these are supposed to happen for reverts after some length of
time:
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true



>
> https://codereview.chromium.org/1412923004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[spang, 2015-10-29 17:54:40]

Description was changed from

==========
Revert of Move gpu memory calculations to Compositor. (patchset #11 id:220001 of
https://codereview.chromium.org/1324413003/ )

Reason for revert:
Hangs on Chrome OS when rotating the display.

BUG=546653

Original issue's description:
> Move gpu memory calculations to Compositor.
> 
> This moves gpu memory policy code from gpumemorymanager to renderer
compositor.
> It will be useful for renderer to use off screen context.
> 
> BUG=526196
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/127a06b45820e30f78079814557cfacd038d6167
> Cr-Commit-Position: refs/heads/master@{#352301}

TBR=dcheng@chromium.org,boliu@chromium.org,ccameron@chromium.org,danakj@chrom...
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=526196
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Revert of Move gpu memory calculations to Compositor. (patchset #11 id:220001 of
https://codereview.chromium.org/1324413003/ )

Reason for revert:
Hangs on Chrome OS when rotating the display.

BUG=546653

Original issue's description:
> Move gpu memory calculations to Compositor.
> 
> This moves gpu memory policy code from gpumemorymanager to renderer
compositor.
> It will be useful for renderer to use off screen context.
> 
> BUG=526196
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
> 
> Committed: https://crrev.com/127a06b45820e30f78079814557cfacd038d6167
> Cr-Commit-Position: refs/heads/master@{#352301}

TBR=dcheng@chromium.org,boliu@chromium.org,ccameron@chromium.org,danakj@chrom...
BUG=526196
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[spang, 2015-10-29 17:54:46]

The CQ bit was unchecked by spang@chromium.org

[spang, 2015-10-29 17:54:46]

The CQ bit was checked by spang@chromium.org

[spang, 2015-10-29 17:55:14]

On 2015/10/29 17:51:16, danakj wrote:
> On Thu, Oct 29, 2015 at 10:49 AM, <mailto:boliu@chromium.org> wrote:
> 
> > Rebase not completely correct here, since I removed some files that was
> > touched.
> > I can sort through it after though.
> >
> > Although this is a huge patch being reverted after 3+ weeks. Pretty
> > dangerous by
> > itself even if it was clean..
> >
> 
> I don't think these are supposed to happen for reverts after some length of
> time:
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true

You'll notice it doesn't say NOTRY and is running the tryjobs. I removed the
others.

> 
> 
> 
> >
> > https://codereview.chromium.org/1412923004/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2015-10-29 17:56:35]

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

[danakj, 2015-10-29 17:57:14]

On Thu, Oct 29, 2015 at 10:55 AM, <spang@chromium.org> wrote:

> On 2015/10/29 17:51:16, danakj wrote:
>
>> On Thu, Oct 29, 2015 at 10:49 AM, <mailto:boliu@chromium.org> wrote:
>>
>
> > Rebase not completely correct here, since I removed some files that was
>> > touched.
>> > I can sort through it after though.
>> >
>> > Although this is a huge patch being reverted after 3+ weeks. Pretty
>> > dangerous by
>> > itself even if it was clean..
>> >
>>
>
> I don't think these are supposed to happen for reverts after some length of
>> time:
>> NOPRESUBMIT=true
>> NOTREECHECKS=true
>> NOTRY=true
>>
>
> You'll notice it doesn't say NOTRY and is running the tryjobs. I removed
> the
> others.
>

Oh ok, I just read the email. Sounds like bo has some feedback that should
be addressed before landing though?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[spang, 2015-10-29 17:58:59]

On 2015/10/29 17:49:51, boliu wrote:
> Rebase not completely correct here, since I removed some files that was
touched.
> I can sort through it after though.
> 
> Although this is a huge patch being reverted after 3+ weeks. Pretty dangerous
by
> itself even if it was clean..

If there's a real issue, please uncheck commit and we'll deal with it.

[spang, 2015-10-29 17:59:12]

The CQ bit was unchecked by spang@chromium.org

[spang, 2015-10-29 17:59:40]

On 2015/10/29 17:57:14, danakj wrote:
> On Thu, Oct 29, 2015 at 10:55 AM, <mailto:spang@chromium.org> wrote:
> 
> > On 2015/10/29 17:51:16, danakj wrote:
> >
> >> On Thu, Oct 29, 2015 at 10:49 AM, <mailto:boliu@chromium.org> wrote:
> >>
> >
> > > Rebase not completely correct here, since I removed some files that was
> >> > touched.
> >> > I can sort through it after though.
> >> >
> >> > Although this is a huge patch being reverted after 3+ weeks. Pretty
> >> > dangerous by
> >> > itself even if it was clean..
> >> >
> >>
> >
> > I don't think these are supposed to happen for reverts after some length of
> >> time:
> >> NOPRESUBMIT=true
> >> NOTREECHECKS=true
> >> NOTRY=true
> >>
> >
> > You'll notice it doesn't say NOTRY and is running the tryjobs. I removed
> > the
> > others.
> >
> 
> Oh ok, I just read the email. Sounds like bo has some feedback that should
> be addressed before landing though?

Yeah ok, unchecked.

> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[boliu, 2015-10-29 18:05:54]

The issue is I removed
content/browser/android/in_process/synchronous_compositor_context_provider.cc/h
since the orignal patch landed, and now it needs to be brought back, but in
content/renderer/android. I can sort it out later after the revert, or you can
try to bring it back, up to you.

[spang, 2015-10-29 18:30:46]

On 2015/10/29 18:05:54, boliu wrote:
> The issue is I removed
>
content/browser/android/in_process/synchronous_compositor_context_provider.cc/h
> since the orignal patch landed, and now it needs to be brought back, but in
> content/renderer/android. I can sort it out later after the revert, or you can
> try to bring it back, up to you.

Hm maybe you should pick up this revert then. Or maybe we can figure out the
issue but I'd guess the release manager want to stop the bleeding soon.

[boliu, 2015-10-29 18:35:03]

On 2015/10/29 18:30:46, spang wrote:
> On 2015/10/29 18:05:54, boliu wrote:
> > The issue is I removed
> >
>
content/browser/android/in_process/synchronous_compositor_context_provider.cc/h
> > since the orignal patch landed, and now it needs to be brought back, but in
> > content/renderer/android. I can sort it out later after the revert, or you
can
> > try to bring it back, up to you.
> 
> Hm maybe you should pick up this revert then. Or maybe we can figure out the
> issue but I'd guess the release manager want to stop the bleeding soon.

Err, kinda swamped at the moment. I can get to it maybe next week. this lgtm to
land for the synchronous_compositor bits, and I can sort this out after

Looks like lots of red trybots though

[spang, 2015-10-29 18:44:14]

On 2015/10/29 18:35:03, boliu wrote:
> On 2015/10/29 18:30:46, spang wrote:
> > On 2015/10/29 18:05:54, boliu wrote:
> > > The issue is I removed
> > >
> >
>
content/browser/android/in_process/synchronous_compositor_context_provider.cc/h
> > > since the orignal patch landed, and now it needs to be brought back, but
in
> > > content/renderer/android. I can sort it out later after the revert, or you
> can
> > > try to bring it back, up to you.
> > 
> > Hm maybe you should pick up this revert then. Or maybe we can figure out the
> > issue but I'd guess the release manager want to stop the bleeding soon.
> 
> Err, kinda swamped at the moment. I can get to it maybe next week. this lgtm
to
> land for the synchronous_compositor bits, and I can sort this out after
> 
> Looks like lots of red trybots though

Yeah, perhaps it really is too late to revert and we should focus on diagnosing
the issue.

[piman, 2015-10-29 23:49:13]

Several patches landed on top of the assumptions in this patch (among others
https://codereview.chromium.org/1368583006 ), so just reverting this one will
probably make things worse.

[piman, 2015-10-29 23:50:17]

On 2015/10/29 23:49:13, piman (slow to review) wrote:
> Several patches landed on top of the assumptions in this patch (among others
> https://codereview.chromium.org/1368583006 ), so just reverting this one will
> probably make things worse.

(as evidenced by the failing tests)

[spang, 2015-10-29 23:56:58]

On 2015/10/29 23:50:17, piman (slow to review) wrote:
> On 2015/10/29 23:49:13, piman (slow to review) wrote:
> > Several patches landed on top of the assumptions in this patch (among others
> > https://codereview.chromium.org/1368583006 ), so just reverting this one
will
> > probably make things worse.
> 
> (as evidenced by the failing tests)

Yeah I noticed :)

I've closed this since it doesn't seem like reverting is workable at this point.