Attachment 153596 Details for Bug 76225 – Patch

[patch] Patch

bug-76225-20120720140111.patch (text/plain), 11.16 KB, created by Kenneth Russell on 2012-07-20 14:01:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Kenneth Russell

Created: 2012-07-20 14:01:15 PDT

Size: 11.16 KB

patch

obsolete

>Subversion Revision: 123254
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index da379b0f0f5b9615db98aa424ea6ea1a6c1df41e..464e19312ab2a0904ac1fcbd638b7118fa38000c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,46 @@
>+2012-07-20  Kenneth Russell  <kbr@google.com>
>+
>+        [chromium] Hint garbage collector to run if page uses Canvas contexts
>+        https://bugs.webkit.org/show_bug.cgi?id=76225
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Upon creating a canvas context, set a hint in the current isolate
>+        indicating that a full GC should be done upon the next page
>+        navigation.
>+
>+        This improves Chrome's robustness on some WebGL stress tests which
>+        simulate real-world behavior by repeatedly navigating among
>+        several samples. More general measures are being investigated, but
>+        this change makes V8 behave the same as JSC on these stress tests.
>+        JSC doesn't currently use generational garbage collection, so it
>+        has more opportunities to discover unreferenced canvas contexts.
>+
>+        Test: fast/canvas/webgl/context-creation-and-destruction.html
>+
>+        * bindings/v8/V8Binding.cpp:
>+        (WebCore::V8BindingPerIsolateData::V8BindingPerIsolateData):
>+        (WebCore::V8BindingPerIsolateData::setLowMemoryNotificationHint):
>+            Set a per-isolate hint to signal a low memory condition upon the next page navigation.
>+        (WebCore):
>+        (WebCore::V8BindingPerIsolateData::getLowMemoryNotificationHint):
>+            Get the previously set hint.
>+        * bindings/v8/V8Binding.h:
>+        (V8BindingPerIsolateData):
>+            Add getter and setter for low memory notification hint.
>+        * bindings/v8/V8Proxy.cpp:
>+        (WebCore::V8Proxy::hintForGCIfNecessary):
>+            If necessary, send V8 a hint that it should GC.
>+        (WebCore):
>+        (WebCore::V8Proxy::clearForClose):
>+        (WebCore::V8Proxy::clearForNavigation):
>+            Call hintForGCIfNecessary.
>+        * bindings/v8/V8Proxy.h:
>+        (V8Proxy):
>+        * bindings/v8/custom/V8HTMLCanvasElementCustom.cpp:
>+        (WebCore::V8HTMLCanvasElement::getContextCallback):
>+            Set a hint that we should GC upon the next page navigation.
>+
> 2012-07-20  Han Shen  <shenhan@google.com>
> 
>         [Chromium] Compilation fails under gcc 4.7
>diff --git a/Source/WebCore/bindings/v8/V8Binding.cpp b/Source/WebCore/bindings/v8/V8Binding.cpp
>index d282b0dbef786993425b9d76b077a33d036acf13..16591263942e363ef755fcc4667b76cd722c5f6a 100644
>--- a/Source/WebCore/bindings/v8/V8Binding.cpp
>+++ b/Source/WebCore/bindings/v8/V8Binding.cpp
>@@ -60,6 +60,7 @@ V8BindingPerIsolateData::V8BindingPerIsolateData(v8::Isolate* isolate)
> #ifndef NDEBUG
>     , m_internalScriptRecursionLevel(0)
> #endif
>+    , m_lowMemoryNotificationHint(false)
> {
> }
> 
>@@ -102,6 +103,16 @@ void V8BindingPerIsolateData::reportMemoryUsage(MemoryObjectInfo* memoryObjectIn
>         info.addInstrumentedMember(m_domDataList[i]);
> }
> 
>+void V8BindingPerIsolateData::setLowMemoryNotificationHint(bool hint)
>+{
>+    m_lowMemoryNotificationHint = hint;
>+}
>+
>+bool V8BindingPerIsolateData::getLowMemoryNotificationHint() const
>+{
>+    return m_lowMemoryNotificationHint;
>+}
>+
> // WebCoreStringResource is a helper class for v8ExternalString. It is used
> // to manage the life-cycle of the underlying buffer of the external string.
> class WebCoreStringResource : public v8::String::ExternalStringResource {
>diff --git a/Source/WebCore/bindings/v8/V8Binding.h b/Source/WebCore/bindings/v8/V8Binding.h
>index c63bf21b88aafec71e261ceb33135e9c65e898e4..30b6c5ceb5154dc02a41e0fb489abaafd2aea453 100644
>--- a/Source/WebCore/bindings/v8/V8Binding.h
>+++ b/Source/WebCore/bindings/v8/V8Binding.h
>@@ -220,6 +220,13 @@ namespace WebCore {
> 
>         void reportMemoryUsage(MemoryObjectInfo*) const;
> 
>+        // Gives the system a hint that we should send a low memory
>+        // notification upon the next close or navigation event,
>+        // because some expensive objects have been allocated that we
>+        // want to take every opportunity to collect.
>+        void setLowMemoryNotificationHint(bool);
>+        bool getLowMemoryNotificationHint() const;
>+
>     private:
>         explicit V8BindingPerIsolateData(v8::Isolate*);
>         ~V8BindingPerIsolateData();
>@@ -248,6 +255,8 @@ namespace WebCore {
>         int m_internalScriptRecursionLevel;
> #endif
>         GCEventData m_gcEventData;
>+
>+        bool m_lowMemoryNotificationHint;
>     };
> 
>     class ConstructorMode {
>diff --git a/Source/WebCore/bindings/v8/V8Proxy.cpp b/Source/WebCore/bindings/v8/V8Proxy.cpp
>index e4079cdd13408432e4ac3c63f65e3231af0be385..021254e634448967a70c86e76f858e4e0dabaec0 100644
>--- a/Source/WebCore/bindings/v8/V8Proxy.cpp
>+++ b/Source/WebCore/bindings/v8/V8Proxy.cpp
>@@ -559,15 +559,26 @@ void V8Proxy::resetIsolatedWorlds()
>     m_isolatedWorldSecurityOrigins.clear();
> }
> 
>+void V8Proxy::hintForGCIfNecessary()
>+{
>+    V8BindingPerIsolateData* data = V8BindingPerIsolateData::current();
>+    if (data->getLowMemoryNotificationHint()) {
>+        data->setLowMemoryNotificationHint(false);
>+        v8::V8::LowMemoryNotification();
>+    }
>+}
>+
> void V8Proxy::clearForClose()
> {
>     resetIsolatedWorlds();
>+    hintForGCIfNecessary();
>     windowShell()->clearForClose();
> }
> 
> void V8Proxy::clearForNavigation()
> {
>     resetIsolatedWorlds();
>+    hintForGCIfNecessary();
>     windowShell()->clearForNavigation();
> }
> 
>diff --git a/Source/WebCore/bindings/v8/V8Proxy.h b/Source/WebCore/bindings/v8/V8Proxy.h
>index 64fc4e6694ab2ddf8ba9a22d4a61ec81bed32638..7d40077ecbf533e312f72e30e499de01cdfac639 100644
>--- a/Source/WebCore/bindings/v8/V8Proxy.h
>+++ b/Source/WebCore/bindings/v8/V8Proxy.h
>@@ -271,6 +271,8 @@ namespace WebCore {
>     private:
>         void resetIsolatedWorlds();
> 
>+        void hintForGCIfNecessary();
>+
>         PassOwnPtr<v8::ScriptData> precompileScript(v8::Handle<v8::String>, CachedScript*);
> 
>         static const char* rangeExceptionName(int exceptionCode);
>diff --git a/Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp b/Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp
>index e19896fee0201b1b54b355d9d0cb2183fd2789e4..9e4b2b050f402892a79cf8766f7fb156675fc9c8 100644
>--- a/Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp
>+++ b/Source/WebCore/bindings/v8/custom/V8HTMLCanvasElementCustom.cpp
>@@ -86,6 +86,13 @@ v8::Handle<v8::Value> V8HTMLCanvasElement::getContextCallback(const v8::Argument
>     CanvasRenderingContext* result = imp->getContext(contextId, attrs.get());
>     if (!result)
>         return v8::Null(args.GetIsolate());
>+
>+    // Both 2D and 3D canvas contexts can hold on to lots of GPU resources, and we
>+    // want to take an opportunity to get rid of them as soon as possible when we
>+    // navigate away from pages using them.
>+    V8BindingPerIsolateData* perIsolateData = V8BindingPerIsolateData::current(args.GetIsolate());
>+    perIsolateData->setLowMemoryNotificationHint(true);
>+
>     if (result->is2d())
>         return toV8(static_cast<CanvasRenderingContext2D*>(result), args.GetIsolate());
> #if ENABLE(WEBGL)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fbd82be63dc61a32584f30c845e0b477f8b3fe91..7bd33ea9ecc29b3b705795a371efb99059f95017 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2012-07-20  Kenneth Russell  <kbr@google.com>
>+
>+        [chromium] Hint garbage collector to run if page uses Canvas contexts
>+        https://bugs.webkit.org/show_bug.cgi?id=76225
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This test doesn't directly exercise this code path yet, but it
>+        needs to work regardless and also needs to be expanded.
>+
>+        * fast/canvas/webgl/context-creation-and-destruction-expected.txt: Added.
>+        * fast/canvas/webgl/context-creation-and-destruction.html: Added.
>+
> 2012-07-20  Tony Chang  <tony@chromium.org>
> 
>         [chromium] Rebaseline css3/filters/custom tests
>diff --git a/LayoutTests/fast/canvas/webgl/context-creation-and-destruction-expected.txt b/LayoutTests/fast/canvas/webgl/context-creation-and-destruction-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..118eba49bc1fd4e474be3e892c97a15140b4f654
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/webgl/context-creation-and-destruction-expected.txt
>@@ -0,0 +1,24 @@
>+Test that contexts are freed and garbage collected reasonably
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+test 1 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 2 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 3 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 4 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 5 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 6 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 7 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+test 8 of 8
>+PASS getError was expected value: NO_ERROR : Should be no errors
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/canvas/webgl/context-creation-and-destruction.html b/LayoutTests/fast/canvas/webgl/context-creation-and-destruction.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..64b6d44f55caa46fd4b9baa2e009d73cc7b12f7a
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/webgl/context-creation-and-destruction.html
>@@ -0,0 +1,60 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<link rel="stylesheet" href="../../js/resources/js-test-style.css"/>
>+<script src="../../js/resources/js-test-pre.js"></script>
>+<script src="resources/webgl-test.js"> </script>
>+<script src="resources/webgl-test-utils.js"> </script>
>+</head>
>+<body>
>+<div id="description"></div>
>+<div id="console"></div>
>+<script>
>+var wtu = WebGLTestUtils;
>+// Ideally we would make this test run far longer, but we need to keep
>+// it within a reasonable per-test timeout.
>+var target = 8;
>+var count = 0;
>+
>+if (window.initNonKhronosFramework) {
>+    window.initNonKhronosFramework(true);
>+}
>+
>+description('Test that contexts are freed and garbage collected reasonably');
>+doNextTest();
>+
>+// Creates a canvas and some textures then exits. There are
>+// no references to either so all should be garbage collected.
>+function test() {
>+  var canvas = document.createElement("canvas");
>+  // This is safe for any device. See drawingBufferWidth in spec.
>+  canvas.width = 2048;
>+  canvas.height = 2048;
>+  var gl = wtu.create3DContext(canvas);
>+  var maxTextureSize = gl.getParameter(gl.MAX_TEXTURE_SIZE);
>+  var size = Math.min(1024, maxTextureSize);
>+  for (var jj = 0; jj < 5; ++jj) {
>+    var tex = gl.createTexture();
>+    gl.bindTexture(gl.TEXTURE_2D, tex);
>+    gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, size, size, 0, gl.RGBA, gl.UNSIGNED_BYTE,
>+                  new Uint8Array(size * size * 4));
>+  }
>+  gl.clear(gl.COLOR_BUFFER_BIT);
>+  glErrorShouldBe(gl, gl.NO_ERROR, "Should be no errors");
>+}
>+
>+function doNextTest() {
>+  ++count;
>+  debug("test " + count + " of " + target);
>+  test();
>+  if (count < target) {
>+    setTimeout(doNextTest, 100);
>+  } else {
>+    finishTest();
>+  }
>+}
>+</script>
>+
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 76225: 152094 | 152130 | 153596 | 153872 | 153923 | 154146