New issue
Advanced search Search tips

Issue 832197 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 671916



Sign in to add a comment

MacViewsBrowser: Excessive memory use per window

Project Member Reported by sdy@chromium.org, Apr 12 2018

Issue description

Chrome Version: 67.0.3395.0
OS: macOS 10.13.3

What steps will reproduce the problem?
(1) Open a few windows in MacViews mode.

What is the expected result?
Slight browser process memory increase per window. (~7MB in Cocoa)

What happens instead?
~100MB increase per window.

I suspect this is at least in part because some window-sized Cocoa views use drawRect:, which gives them a backing store, and should not, but other things may be happening too.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d18ebb63085f1278540a0d1970be93be2a6265d

commit 9d18ebb63085f1278540a0d1970be93be2a6265d
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Apr 13 00:15:16 2018

Delete an (expensive) unused code path to draw window backgrounds.

Any view that implements drawRect: and/or sets needsDisplay on itself
gets a view-sized backing store. This was costing ~100MB per large
window on my MBP, and nothing was using it: 10.9 is no longer supported
and menu backgrounds are no longer blurred (and if they were, it'd be a
different way).

Bug:  832197 
Change-Id: I2aeaa53a19c12f23a2e05d33883eabf76e318f1c
Reviewed-on: https://chromium-review.googlesource.com/1011022
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550429}
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_content_view.h
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_native_widget.mm

Comment 2 by sdy@chromium.org, Apr 16 2018

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d18ebb63085f1278540a0d1970be93be2a6265d

commit 9d18ebb63085f1278540a0d1970be93be2a6265d
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Apr 13 00:15:16 2018

Delete an (expensive) unused code path to draw window backgrounds.

Any view that implements drawRect: and/or sets needsDisplay on itself
gets a view-sized backing store. This was costing ~100MB per large
window on my MBP, and nothing was using it: 10.9 is no longer supported
and menu backgrounds are no longer blurred (and if they were, it'd be a
different way).

Bug:  832197 
Change-Id: I2aeaa53a19c12f23a2e05d33883eabf76e318f1c
Reviewed-on: https://chromium-review.googlesource.com/1011022
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550429}
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_content_view.h
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/9d18ebb63085f1278540a0d1970be93be2a6265d/ui/views/cocoa/bridged_native_widget.mm

Labels: Needs-Feedback
Able to reproduce this issue on build without fix hence verifying on latest canary 68.0.3419.0 using Mac 10.12.6

In 67.0.3395.0 -- observing memory in task manager as 109MB.
In 68.0.3419.0 -- observing memory in task manager as 72.6MB. Attaching screenshots for reference.

@sdy: Could you please let us know whether this fix is working as intended or not.


832917_M67.png
153 KB View Download
832197_M68.png
243 KB View Download

Comment 5 by sdy@chromium.org, May 4 2018

Thanks for the attempt to verify. Could you try again with Activity Monitor instead of the Chrome task manager? I'm actually not sure why the difference doesn't show up in the Chrome task manager. With a few windows the difference should be much larger.

Sign in to add a comment