Do V8 GC ASAP if system memory is pressured

content/renderer/render_thread_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_thread_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <utility>
@@ skipping 145 matching lines @@
 #include "third_party/WebKit/public/web/WebMemoryPressureListener.h"
 #include "third_party/WebKit/public/web/WebNetworkStateNotifier.h"
 #include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
 #include "third_party/WebKit/public/web/WebScriptController.h"
 #include "third_party/WebKit/public/web/WebSecurityPolicy.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "third_party/icu/source/i18n/unicode/timezone.h"
 #include "third_party/skia/include/core/SkGraphics.h"
 #include "ui/base/layout.h"
 #include "ui/base/ui_base_switches.h"
-#include "v8/include/v8.h"
 
 #if defined(OS_ANDROID)
 #include <cpu-features.h>
 #include "content/renderer/android/synchronous_compositor_external_begin_frame_source.h"
 #include "content/renderer/android/synchronous_compositor_factory.h"
 #include "content/renderer/android/synchronous_compositor_filter.h"
 #include "content/renderer/media/android/renderer_demuxer_android.h"
 #include "content/renderer/media/android/stream_texture_factory_impl.h"
 #include "media/base/android/media_codec_util.h"
 #endif
@@ skipping 630 matching lines @@
   media::InitializeMediaLibrary();
 
 #if defined(OS_ANDROID)
   if (!command_line.HasSwitch(switches::kDisableAcceleratedVideoDecode) &&
       media::MediaCodecUtil::IsMediaCodecAvailable()) {
     media::EnablePlatformDecoderSupport();
   }
 #endif
 
   memory_pressure_listener_.reset(new base::MemoryPressureListener(
-      base::Bind(&RenderThreadImpl::OnMemoryPressure, base::Unretained(this))));
+      base::Bind(&RenderThreadImpl::OnMemoryPressure, base::Unretained(this)),
+      base::Bind(&RenderThreadImpl::OnSyncMemoryPressure,
+                 base::Unretained(this))));
 
   int num_raster_threads = 0;
   std::string string_value =
       command_line.GetSwitchValueASCII(switches::kNumRasterThreads);
   bool parsed_num_raster_threads =
       base::StringToInt(string_value, &num_raster_threads);
   DCHECK(parsed_num_raster_threads) << string_value;
   DCHECK_GT(num_raster_threads, 0);
 
   // TODO(vmpstr): If the flag sticks, we should clean it up and always have
@@ skipping 1170 matching lines @@
   new EmbeddedSharedWorkerStub(params.url,
                                params.name,
                                params.content_security_policy,
                                params.security_policy_type,
                                params.pause_on_start,
                                params.route_id);
 }
 
 void RenderThreadImpl::OnMemoryPressure(
     base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+  TRACE_EVENT0("memory","RenderThreadImpl::OnMemoryPressure");
   ReleaseFreeMemory();
 
   // Do not call into blink if it is not initialized.
   if (blink_platform_impl_) {
     blink::WebMemoryPressureListener::onMemoryPressure(
         static_cast<blink::WebMemoryPressureLevel>(memory_pressure_level));
 
     if (blink::mainThreadIsolate()) {
-      // Trigger full v8 garbage collection on memory pressure notifications.
-      // This will potentially hang the renderer for a long time, however, when
-      // we receive a memory pressure notification, we might be about to be
-      // killed.
-      blink::mainThreadIsolate()->LowMemoryNotification();
       RenderThread::Get()->PostTaskToAllWebWorkers(
           base::Bind(&LowMemoryNotificationOnThisThread));
     }
 
     if (memory_pressure_level ==
         base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
       // Purge Skia font cache, by setting it to 0 and then again to the
       // previous limit.
       size_t font_cache_limit = SkGraphics::SetFontCacheLimit(0);
       SkGraphics::SetFontCacheLimit(font_cache_limit);
@@ skipping 138 matching lines @@
 RenderThreadImpl::PendingRenderFrameConnect::~PendingRenderFrameConnect() {
 }
 
 void RenderThreadImpl::PendingRenderFrameConnect::OnConnectionError() {
   size_t erased =
       RenderThreadImpl::current()->pending_render_frame_connects_.erase(
           routing_id_);
   DCHECK_EQ(1u, erased);
 }
 
+void RenderThreadImpl::OnSyncMemoryPressure(
+    base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+  v8::MemoryPressureLevel memoryPressureLevel =

[esprehn, 2016/03/25 09:20:35]

v8_memory_pressure_level or something of that nature, content uses hacker_case
variables.

[hong.zheng, 2016/04/01 09:22:34]

Done.

+      v8::MemoryPressureLevel::kNone;
+  switch (memory_pressure_level) {

[jochen (gone - plz use gerrit), 2016/03/30 16:23:01]

this should probably also take into account whether the renderer is in the
foreground or not?

[hong.zheng, 2016/04/01 09:22:34]

The CL can deliver memory pressure notification to V8 directly and V8 only does
immediate GC for critical notification. Considering that critical notification
means background process will be killed and foreground process also has OOM
risk, such as BUG=590975. So maybe we need deliver memory pressure notification
to V8 in foreground.

[jochen (gone - plz use gerrit), 2016/04/03 11:21:35]

V8 doesn't look at whether the renderer is in the foreground or not. We should
translate a android memory pressure event + (foreground state) to the
appropriate v8 memory pressure event here.

[hong.zheng, 2016/04/05 09:09:17]

It is a problem about how to balance OOM risk and performance. There are two
solutions:
1. use the same memory pressure level as chromium in V8 for foreground renderer
and translate chromium critical and moderate level to V8 critical level for
background renderer. 
2. translate chromium critical and moderate level to V8 moderate level for
foreground renderer and use the same memory pressure level as chromium in V8 for
background renderer
which one do you think more make sense?

[hong.zheng, 2016/04/07 10:55:49]

Done.

+    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
+      memoryPressureLevel = v8::MemoryPressureLevel::kNone;

[esprehn, 2016/03/25 09:20:35]

Sigh, not having base in v8 makes for really silly code. Why does base/ use -1,
0, 2 instead of regular enum indexing for this? If the enums were parallel we
could just static_assert and cast between them.

[hong.zheng, 2016/04/01 09:22:34]

Done.

+      break;
+    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
+      memoryPressureLevel = v8::MemoryPressureLevel::kModerate;
+      break;
+    case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
+      memoryPressureLevel = v8::MemoryPressureLevel::kCritical;
+      break;
+    default:
+      NOTREACHED();
+  }
+  blink::mainThreadIsolate()->MemoryPressureNotification(memoryPressureLevel);

[jochen (gone - plz use gerrit), 2016/03/30 16:22:23]

since this API is threadsafe, why not notify the workers as well?

[hong.zheng, 2016/04/01 09:22:34]

Done.

+}
+
 }  // namespace content