Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1482363004:
Fixing laggy chrome on a multitude of accelerated canvases (Closed)
|
Created:
5 years ago by xlai (Olivia) Modified:
5 years ago CC:
chromium-reviews, dshwang, ajuma+watch-canvas_chromium.org, blink-reviews-html_chromium.org, dglazkov+blink, Rik, blink-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixing laggy chrome on a multitude of accelerated canvases
This CL adds a static variable in ImageBuffer that keeps track of
global GPU memory usage, as well as a member variable for GPU
memory usage in current ImageBuffer. These are useful metrics for
accelerated canvases. By doing local experiments on Linux
desktops and Android One, I put a conservative estimate on the
maximum limit that this static variable can take before the UI
responses on chrome becomes laggy. The estimate is equivalent
to the allocated memory for 80 accelerated 2d canvases with
1000*500 size; the estimate for Android is a quarter of that. The
GPU memory usage should be updated when ImageBuffer is
created, destroyed or switching from accelerated mode to non-
accelerated mode or vice versa. Unit Test CanvasRendering-
Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added.
BUG=548209
Committed: https://crrev.com/03e80a23c15918259bafc6e67c77e7059e377391
Cr-Commit-Position: refs/heads/master@{#364487}
Patch Set 1 #
Total comments: 14
Patch Set 2 : Changes based on feedbacks from danakj #
Total comments: 6
Patch Set 3 : Changes to naming/computation according to feedback #Patch Set 4 : Moving variables to ImageBuffer and Adding Unit Test #
Total comments: 12
Patch Set 5 : Renamed "total" to "global"; changing positions that call updateGPUMemory function; added tests abo… #Patch Set 6 : Rebase and Fixing Presubmit warnings #Patch Set 7 : m_imageBuffer nullibility check to ensure other unrelated unit tests on layerbridge not fail #
Messages
Total messages: 34 (15 generated)
xlai@chromium.org changed reviewers: + danakj@chromium.org, junov@chromium.org
junov@: the original updateExternallyAllocatedMemory computes usage for both un-accelerated and accelerated canvases. I separated the two cases and only add (and check) the amount to the static variable s_totalMemoryForAcceleratedCanvases for accelerated canvases. danakj@: the gpu memory management in canvas is diff from that in compositor. Could you check if there's anything that needs to be improved on the current approach I take?
https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:83: // Max Limit of Externally Allocated Memory comments are sentences, please add periods, and don't need to capitalize everything like a title. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:85: const int MaxTotalExternallyAllocatedMemory = 6000000 * 80; can you write it as like 1000*500*4.. i'm not sure what the extra *3 would be. Maybe the comment can explain the other factors in this number too. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:85: const int MaxTotalExternallyAllocatedMemory = 6000000 * 80; also, naming for contants is kNameOfConstant. but then, the other consts in this file are wrong too so idk. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:616: if (canvasPixelCount >= ExpensiveCanvasHeuristicParameters::PreferDisplayListOverGpuSizeThreshold) This looks like a separate CL, cuz it's an unrelated change to what this CL means to do? https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:628: // compositor starves and browser becomes laggy. Thus, we should stop allocating more GPU memory to don't indent comments extra, all lines get the same whitespace. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:630: if (s_totalMemoryForAcceleratedCanvases >= MaxTotalExternallyAllocatedMemory) You'll go over the budget before you start rejecting. Should you be checking (totalAllocatedMemory + memoryThisCanvasWillUse) instead? https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:632: #if defined(OS_ANDROID) I think you can do this ifdefing around what the limit is instead of putting it in the code here instead. And maybe this can be a platform setting instead of an #ifdef? Not sure how blink does this normally. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLCanvasElement.h (right): https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.h:213: static intptr_t s_totalMemoryForAcceleratedCanvases; I would maybe suggest tying this to a single blink (or canvas) instance rather than the whole process. There may be multiple tabs in the same process, that are all visible. And usually you'd want them to each have their own budget, probably (compositors would). Maybe junov has thoughts here.
Fixed codes/comments/styles based on previous feedbacks. There are a few parts that I didn't change and I explained in the inline comments. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:85: const int MaxTotalExternallyAllocatedMemory = 6000000 * 80; On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > also, naming for contants is kNameOfConstant. but then, the other consts in this > file are wrong too so idk. Refactored all the constants in this file to fit the naming convention. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:616: if (canvasPixelCount >= ExpensiveCanvasHeuristicParameters::PreferDisplayListOverGpuSizeThreshold) On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > This looks like a separate CL, cuz it's an unrelated change to what this CL > means to do? Sorry that's a careless mistake. Had corrected it. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:630: if (s_totalMemoryForAcceleratedCanvases >= MaxTotalExternallyAllocatedMemory) On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > You'll go over the budget before you start rejecting. Should you be checking > (totalAllocatedMemory + memoryThisCanvasWillUse) instead? I think the max limit here is more like a soft limit, not a hard limit. Creating one more accelerated canvas over the limit is fine. We just need to prevent creating lots and lots of accelerated canvases beyond the limit. Also, this line of code is wrapped in the shouldAccelerate() function that is invoked when creating a new image buffer for a freshly created canvas. One important factor in determining the memoryThisCanvasWillUse is whether it is accelerated. So, implementing an estimate within the shouldAccelerate() is kind of a "recursive" logic that might be confusing to readers. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:632: #if defined(OS_ANDROID) On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > I think you can do this ifdefing around what the limit is instead of putting it > in the code here instead. > > And maybe this can be a platform setting instead of an #ifdef? Not sure how > blink does this normally. Acknowledged. Moved the code to the front in setting the max. Also, changed to OS(ANDROID) to detect platform because this is used in other codes in third_party/WebKit. https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLCanvasElement.h (right): https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.h:213: static intptr_t s_totalMemoryForAcceleratedCanvases; On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > I would maybe suggest tying this to a single blink (or canvas) instance rather > than the whole process. There may be multiple tabs in the same process, that are > all visible. And usually you'd want them to each have their own budget, probably > (compositors would). Maybe junov has thoughts here. I did some experiments and confirmed that this variable is keeping track of each browser tab's own usage separately; so it's not a "total" computation of all tabs.
https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLCanvasElement.h (right): https://codereview.chromium.org/1482363004/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLCanvasElement.h:213: static intptr_t s_totalMemoryForAcceleratedCanvases; On 2015/12/01 21:50:25, Olivia wrote: > On 2015/11/30 23:07:03, danakj (behind on reviews) wrote: > > I would maybe suggest tying this to a single blink (or canvas) instance rather > > than the whole process. There may be multiple tabs in the same process, that > are > > all visible. And usually you'd want them to each have their own budget, > probably > > (compositors would). Maybe junov has thoughts here. > > I did some experiments and confirmed that this variable is keeping track of each > browser tab's own usage separately; so it's not a "total" computation of all > tabs. Chrome will put tabs in different processes in different ways, depending how you launched the tab (from a js link = same process) and on diff platforms (on android almost all tabs share one process.. maybe they all do)
https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:72: const int kDefaultWidth = 300; The k prefix is a skia thing. In Blink use CapitalizedCamelCase. https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:640: if (s_totalMemoryForAcceleratedCanvases >= kMaxTotalExternallyAllocatedMemory) Constant should be named in a way that reflects the fact that is is about GPU memeory, and not externally allocated. The notion of "externally allocated" is a V8 concept that has nothing to do with this. https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:808: updateTotalMemoryForAcceleratedCanvases(diffFromCurrAllocatedMemory); This is wrong. You need to track gpu allocated memory independently of externally allocated memory otherwise your total will drift when you have a canvas that switches between accelerated and non-accelerated (for example when it is resized). https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:815: Checked<intptr_t, RecordOverflow> checkedExternallyAllocatedMemory = s_totalMemoryForAcceleratedCanvases; Don't use the term "ExternallyAllocated" here, it is confusing.
@danakj: Why do think the threshold tracking should be a tab-local or page-local thing? GPU memory is a shared system resource, so shouldn't we be tracking it in a way that is as global as reasonably possible?
> @danakj: Why do think the threshold tracking should be a tab-local or page-local > thing? GPU memory is a shared system resource, so shouldn't we be tracking it in > a way that is as global as reasonably possible? For the compositor we tried to make global budgets and give diff amounts of memory to diff compositors, and it was a giant mess and we finally deleted the last of that effort recently years later. We found a fixed budget per-compositor to be just fine, and way simpler to reason about. It's possible canvas has different requirements here, as I'm just going off past experience with compositor. https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:72: const int kDefaultWidth = 300; On 2015/12/02 16:33:33, Justin Novosad wrote: > The k prefix is a skia thing. In Blink use CapitalizedCamelCase. It's also a chromium thing, I didn't know blink has a diff rule currently.
https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:72: const int kDefaultWidth = 300; On 2015/12/02 18:23:40, danakj (behind on reviews) wrote: > On 2015/12/02 16:33:33, Justin Novosad wrote: > > The k prefix is a skia thing. In Blink use CapitalizedCamelCase. > > It's also a chromium thing, I didn't know blink has a diff rule currently. FWIW I only see a blink-specific rule for enums: "Enum members should use InterCaps with an initial capital letter. [names-enum-members]" I guess another thing to converge on though.
On 2015/12/02 18:23:40, danakj (behind on reviews) wrote: > > @danakj: Why do think the threshold tracking should be a tab-local or > page-local > > thing? GPU memory is a shared system resource, so shouldn't we be tracking it > in > > a way that is as global as reasonably possible? > > For the compositor we tried to make global budgets and give diff amounts of > memory to diff compositors, and it was a giant mess and we finally deleted the > last of that effort recently years later. We found a fixed budget per-compositor > to be just fine, and way simpler to reason about. > > It's possible canvas has different requirements here, as I'm just going off past > experience with compositor. In the case of canvases we don't have the option to give more or less budget to pages based on their visibility. Basically, a canvas's image buffer will live on the GPU for it's whole lifetime if it is in accelerated mode, even when the page is backgrounded, and we want to take that into account. This could all change in the future though... > > https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): > > https://codereview.chromium.org/1482363004/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:72: const int > kDefaultWidth = 300; > On 2015/12/02 16:33:33, Justin Novosad wrote: > > The k prefix is a skia thing. In Blink use CapitalizedCamelCase. > > It's also a chromium thing, I didn't know blink has a diff rule currently. Actually, I looked at the style guide, and it seems the rules have been relaxed. Still, there is no mandate to do this, and drive-by changes like this should be avoided because they add noise to the history (unexpected 'git blame' indirections).
Patchset #3 (id:40001) has been deleted
Basic changes: 1)rename updateExternallyAllocatedMemory to updateMemoryUsage() so that it encompasses both non-GPU and GPU memory usage updates. 2)Add in one member variable and one static variable to keep track of GPU memory usage. 3) Add in two new constants.
https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (left): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:765: // Four bytes per pixel per buffer. Why remove this comment? https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:83: // We estimate the max limit of GPU allocated memory for canvases before Chrome becomes laggy by setting the There is no hard rule for this in Blink, but we generally respect 80col for multi-line comments https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:94: const int MaxTotalGPUMemoryUsage = 4000000 * 40; This limit feels excessive https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:632: // compositor starves and browser becomes laggy. Thus, we should stop allocating more GPU memory to wrapping https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLCanvasElement.h (left): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLCanvasElement.h:212: No real changes to this file, please revert. https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp (right): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:83: intptr_t getCurrGPUMemoryUsage() const { return canvasElement().buffer()->getGPUMemoryUsage(); } Curr -> Current https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:179: class FakeAcceleratedImageBufferSurfaceForOverwriteTesting : public UnacceleratedImageBufferSurface { Bad name, this is not for overwrite testing https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:183: bool isAccelerated() const override { return true; } you need a "setIsAccelerated" to go with this in order to test the case where an accelerated surface switches to be non-accelerated. https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp:185: MOCK_METHOD0(willOverwriteCanvas, void()); this is not needed. https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:190: m_imageBuffer->updateGPUMemoryUsage(); Should call this after creating m_layer, otherwise isAccelerated may return the wrong value. https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp (right): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp:367: } In the case where we are not accelerated, you need to bring m_gpuMemoryUsage to 0. You would have caught this if you had a test that checked the case where a surface switches from accelerated to non-accelerated. https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/graphics/ImageBuffer.h (right): https://codereview.chromium.org/1482363004/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/graphics/ImageBuffer.h:139: static intptr_t getTotalGPUMemoryUsage() { return s_totalGPUMemoryUsage; } Total -> Global
A new patchset that makes the following changes: Renamed "total" to "global"; Change "FakeAcceleratedImageBufferSurfaceForTesting" class; changing positions that call updateGPUMemory function; added tests about switching accelerating mode; style (limit comment col number within 80)
lgtm This other WIP CL deals with coordinating GPU resource management with page visibility https://codereview.chromium.org/1507863005/
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in HTMLCanvasElement that keeps track of externally allocated memory ONLY used by accelerated canvases, which is a meaningful representation of the GPU memory usage of canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is half of that. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. BUG=548209 ==========
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. BUG=548209 ==========
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. BUG=548209 ==========
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. Unit Test CanvasRendering- Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added. BUG=548209 ==========
The CQ bit was checked by xlai@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1482363004/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1482363004/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by xlai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org Link to the patchset: https://codereview.chromium.org/1482363004/#ps120001 (title: "Rebase and Fixing Presubmit warnings")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1482363004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1482363004/120001
The CQ bit was unchecked by xlai@chromium.org
The CQ bit was checked by xlai@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org Link to the patchset: https://codereview.chromium.org/1482363004/#ps140001 (title: "m_imageBuffer nullibility check to ensure other unrelated unit tests on layerbridge not fail")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1482363004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1482363004/140001
Message was sent while issue was closed.
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. Unit Test CanvasRendering- Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. Unit Test CanvasRendering- Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added. BUG=548209 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. Unit Test CanvasRendering- Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added. BUG=548209 ========== to ========== Fixing laggy chrome on a multitude of accelerated canvases This CL adds a static variable in ImageBuffer that keeps track of global GPU memory usage, as well as a member variable for GPU memory usage in current ImageBuffer. These are useful metrics for accelerated canvases. By doing local experiments on Linux desktops and Android One, I put a conservative estimate on the maximum limit that this static variable can take before the UI responses on chrome becomes laggy. The estimate is equivalent to the allocated memory for 80 accelerated 2d canvases with 1000*500 size; the estimate for Android is a quarter of that. The GPU memory usage should be updated when ImageBuffer is created, destroyed or switching from accelerated mode to non- accelerated mode or vice versa. Unit Test CanvasRendering- Context2DTest.GPUMemoryUpdateForAcceleratedCanvas added. BUG=548209 Committed: https://crrev.com/03e80a23c15918259bafc6e67c77e7059e377391 Cr-Commit-Position: refs/heads/master@{#364487} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/03e80a23c15918259bafc6e67c77e7059e377391 Cr-Commit-Position: refs/heads/master@{#364487} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
