Skip to content

Commit

Permalink
content: Add asyncCollectGarbage to GCController
Browse files Browse the repository at this point in the history
Adds an async GC call to GCController which is used from asyncGC() during layout
tests.

Currently the async GC calls a regular V8 GC which schedules a precise Oilpan GC
as followup. In a unified GC world a V8 GC runs both, V8's GC and Oilpan, with
the difference that it has to conservatively scan the stack as Oilpan is not run
from the event loop. This is fixed by posting a task and calling a V8 GC
indicating that there's no relevant stack present.

Bug: chromium:843903
Change-Id: I411d63df4aa61b51928aded86d998ad78af35a1e
Reviewed-on: https://chromium-review.googlesource.com/1236074
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593503}
  • Loading branch information
mlippautz authored and Commit Bot committed Sep 24, 2018
1 parent 4a452f4 commit 5b64e89
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 22 deletions.
56 changes: 43 additions & 13 deletions content/shell/test_runner/gc_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "content/shell/test_runner/gc_controller.h"

#include "content/shell/test_runner/test_interfaces.h"
#include "content/shell/test_runner/web_test_delegate.h"
#include "gin/arguments.h"
#include "gin/handle.h"
#include "gin/object_template_builder.h"
Expand All @@ -16,7 +18,8 @@ namespace test_runner {
gin::WrapperInfo GCController::kWrapperInfo = {gin::kEmbedderNativeGin};

// static
void GCController::Install(blink::WebLocalFrame* frame) {
void GCController::Install(TestInterfaces* interfaces,
blink::WebLocalFrame* frame) {
v8::Isolate* isolate = blink::MainThreadIsolate();
v8::HandleScope handle_scope(isolate);
v8::Local<v8::Context> context = frame->MainWorldScriptContext();
Expand All @@ -26,23 +29,25 @@ void GCController::Install(blink::WebLocalFrame* frame) {
v8::Context::Scope context_scope(context);

gin::Handle<GCController> controller =
gin::CreateHandle(isolate, new GCController());
gin::CreateHandle(isolate, new GCController(interfaces));
if (controller.IsEmpty())
return;
v8::Local<v8::Object> global = context->Global();
global->Set(gin::StringToV8(isolate, "GCController"), controller.ToV8());
}

GCController::GCController() {}
GCController::GCController(TestInterfaces* interfaces)
: interfaces_(interfaces) {}

GCController::~GCController() {}
GCController::~GCController() = default;

gin::ObjectTemplateBuilder GCController::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return gin::Wrappable<GCController>::GetObjectTemplateBuilder(isolate)
.SetMethod("collect", &GCController::Collect)
.SetMethod("collectAll", &GCController::CollectAll)
.SetMethod("minorCollect", &GCController::MinorCollect);
.SetMethod("minorCollect", &GCController::MinorCollect)
.SetMethod("asyncCollectAll", &GCController::AsyncCollectAll);
}

void GCController::Collect(const gin::Arguments& args) {
Expand All @@ -51,19 +56,44 @@ void GCController::Collect(const gin::Arguments& args) {
}

void GCController::CollectAll(const gin::Arguments& args) {
// In order to collect a DOM wrapper, two GC cycles are needed.
// In the first GC cycle, a weak callback of the DOM wrapper is called back
// and the weak callback disposes a persistent handle to the DOM wrapper.
// In the second GC cycle, the DOM wrapper is reclaimed.
// Given that two GC cycles are needed to collect one DOM wrapper,
// more than two GC cycles are needed to collect all DOM wrappers
// that are chained. Seven GC cycles look enough in most tests.
for (int i = 0; i < 7; i++) {
for (int i = 0; i < kNumberOfGCsForFullCollection; i++) {
args.isolate()->RequestGarbageCollectionForTesting(
v8::Isolate::kFullGarbageCollection);
}
}

void GCController::AsyncCollectAll(const gin::Arguments& args) {
if (args.PeekNext().IsEmpty()) {
NOTREACHED() << "AsyncCollectAll should be called with callback argument.";
}

v8::HandleScope scope(args.isolate());
v8::UniquePersistent<v8::Function> func(
args.isolate(), v8::Local<v8::Function>::Cast(args.PeekNext()));

CHECK(interfaces_->GetDelegate());
CHECK(!func.IsEmpty());
interfaces_->GetDelegate()->PostTask(
base::BindOnce(&GCController::AsyncCollectAllWithEmptyStack,
base::Unretained(this), std::move(func)));
}

void GCController::AsyncCollectAllWithEmptyStack(
v8::UniquePersistent<v8::Function> callback) {
v8::Isolate* const isolate = blink::MainThreadIsolate();

for (int i = 0; i < kNumberOfGCsForFullCollection; i++) {
isolate->GetEmbedderHeapTracer()->GarbageCollectionForTesting(
v8::EmbedderHeapTracer::kEmpty);
}

v8::HandleScope scope(isolate);
v8::Local<v8::Function> func = callback.Get(isolate);
v8::Local<v8::Context> context = func->CreationContext();
v8::Context::Scope context_scope(context);
func->Call(context, v8::Undefined(isolate), 0, nullptr).ToLocalChecked();
}

void GCController::MinorCollect(const gin::Arguments& args) {
args.isolate()->RequestGarbageCollectionForTesting(
v8::Isolate::kMinorGarbageCollection);
Expand Down
20 changes: 18 additions & 2 deletions content/shell/test_runner/gc_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,23 @@ class Arguments;

namespace test_runner {

class TestInterfaces;

class GCController : public gin::Wrappable<GCController> {
public:
static gin::WrapperInfo kWrapperInfo;
static void Install(blink::WebLocalFrame* frame);
static void Install(TestInterfaces* interfaces, blink::WebLocalFrame* frame);

private:
GCController();
// In the first GC cycle, a weak callback of the DOM wrapper is called back
// and the weak callback disposes a persistent handle to the DOM wrapper.
// In the second GC cycle, the DOM wrapper is reclaimed.
// Given that two GC cycles are needed to collect one DOM wrapper,
// more than two GC cycles are needed to collect all DOM wrappers
// that are chained. Seven GC cycles look enough in most tests.
static constexpr int kNumberOfGCsForFullCollection = 7;

explicit GCController(TestInterfaces* interfaces);
~GCController() override;

// gin::Wrappable.
Expand All @@ -33,8 +43,14 @@ class GCController : public gin::Wrappable<GCController> {

void Collect(const gin::Arguments& args);
void CollectAll(const gin::Arguments& args);
void AsyncCollectAll(const gin::Arguments& args);
void MinorCollect(const gin::Arguments& args);

void AsyncCollectAllWithEmptyStack(
v8::UniquePersistent<v8::Function> callback);

TestInterfaces* interfaces_;

DISALLOW_COPY_AND_ASSIGN(GCController);
};

Expand Down
2 changes: 1 addition & 1 deletion content/shell/test_runner/test_interfaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void TestInterfaces::SetDelegate(WebTestDelegate* delegate) {

void TestInterfaces::BindTo(blink::WebLocalFrame* frame) {
gamepad_controller_->Install(frame);
GCController::Install(frame);
GCController::Install(this, frame);
}

void TestInterfaces::ResetTestHelperControllers() {
Expand Down
4 changes: 2 additions & 2 deletions gin/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class GIN_EXPORT Arguments {
explicit Arguments(const v8::FunctionCallbackInfo<v8::Value>& info);
~Arguments();

template<typename T>
bool GetHolder(T* out) {
template <typename T>
bool GetHolder(T* out) const {
return ConvertFromV8(isolate_, info_->Holder(), out);
}

Expand Down
6 changes: 2 additions & 4 deletions third_party/WebKit/LayoutTests/resources/js-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,15 +752,13 @@ function asyncGC(callback) {
return new Promise(resolve => asyncGC(resolve));
}
var documentsBefore = internals.numberOfLiveDocuments();
GCController.collectAll();
// FIXME: we need a better way of waiting for chromium events to happen
setTimeout(function () {
GCController.asyncCollectAll(function () {
var documentsAfter = internals.numberOfLiveDocuments();
if (documentsAfter < documentsBefore)
asyncGC(callback);
else
callback();
}, 0);
});
}

function gc() {
Expand Down

0 comments on commit 5b64e89

Please sign in to comment.