Commit af4c4b04 authored by oth's avatar oth Committed by Commit bot

Reland 'Additional HandleScopes to limit Handle consumption.'

v8 builds with --no-snap were hitting handle limits compiling natives for handle count unit tests that run with --check_handle_count. Patch now has higher handle limits (~4k more than required for failing natives compilation).

Original issue: https://codereview.chromium.org/1185633002/

Original issue's description:
    > Additional HandleScopes to limit Handle consumption.
    >
    > erikcorry@chromium.org suggested digging into v8 handle usage. Found potential scopes in ast.cc and runtime-literals.cc and added tests.
    >
    > The runtime-literals.cc change reduces peak handles in imaging-darkroom.js from 1,282,610 to 428,218. The ast.cc change reduces the peak handles in string-t
agcloud.js from 80,738 to 8,176.
    >
    > No significant handle count issues found with major websites, but substantial savings on some benchmarks and demos:
    >
    > Kraken's imaging-darkroom.js down from 1,282,610 to 428,218 due to runtime-literals.cc scope.
    > SunSpider's string-tagcloud.js down from 80,738 to 8.176 due to ast.cc
    >
    > http://www.flohofwoe.net/demos/dragons_asmjs.html (738,906 -> 478,296)
    > http://www.flohofwoe.net/demos/instancing_asmjs.html (737,884 -> 477,274)
    > https://dl.dropboxusercontent.com/u/16662598/Ports/DOSBox-web/doom.html?engine=dosbox-growth.js (1,724,114 -> 1,087,408)
    > https://kripken.github.io/ammo.js/examples/new/ammo.html (175,784 -> 142,058)
    >
    > BUG=
    >
    > Committed: https://crrev.com/3a4c7538839186aa38910c66c986abb563f4ccd2
    > Cr-Commit-Position: refs/heads/master@{#29155}

BUG=

Review URL: https://codereview.chromium.org/1192743005

Cr-Commit-Position: refs/heads/master@{#29322}
parent 572cac67
......@@ -525,8 +525,10 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
depth_acc = m_literal->depth() + 1;
}
}
Handle<Object> boilerplate_value = GetBoilerplateValue(element, isolate);
// New handle scope here, needs to be after BuildContants().
HandleScope scope(isolate);
Handle<Object> boilerplate_value = GetBoilerplateValue(element, isolate);
if (boilerplate_value->IsTheHole()) {
is_holey = true;
continue;
......
......@@ -92,7 +92,19 @@ HandleScope::HandleScope(Isolate* isolate) {
HandleScope::~HandleScope() {
CloseScope(isolate_, prev_next_, prev_limit_);
#ifdef DEBUG
if (FLAG_check_handle_count) {
int before = NumberOfHandles(isolate_);
CloseScope(isolate_, prev_next_, prev_limit_);
int after = NumberOfHandles(isolate_);
DCHECK(after - before < kCheckHandleThreshold);
DCHECK(before < kCheckHandleThreshold);
} else {
#endif // DEBUG
CloseScope(isolate_, prev_next_, prev_limit_);
#ifdef DEBUG
}
#endif // DEBUG
}
......
......@@ -226,6 +226,11 @@ class HandleScope {
Isolate* isolate() { return isolate_; }
// Limit for number of handles with --check_handle-count. This is
// large enough to compile natives and pass unit tests with some
// slack for future changes to natives.
static const int kCheckHandleThreshold = 30 * 1024;
private:
// Prevent heap allocation or illegal handle scopes.
HandleScope(const HandleScope&);
......
......@@ -646,8 +646,8 @@ void Heap::GarbageCollectionEpilogue() {
if (FLAG_print_handles) PrintHandles();
if (FLAG_gc_verbose) Print();
if (FLAG_code_stats) ReportCodeStatistics("After GC");
#endif
if (FLAG_check_handle_count) CheckHandleCount();
#endif
if (FLAG_deopt_every_n_garbage_collections > 0) {
// TODO(jkummerow/ulan/jarin): This is not safe! We can't assume that
// the topmost optimized frame can be deoptimized safely, because it
......@@ -5991,7 +5991,9 @@ void Heap::PrintHandles() {
class CheckHandleCountVisitor : public ObjectVisitor {
public:
CheckHandleCountVisitor() : handle_count_(0) {}
~CheckHandleCountVisitor() { CHECK_LT(handle_count_, 2000); }
~CheckHandleCountVisitor() {
CHECK(handle_count_ < HandleScope::kCheckHandleThreshold);
}
void VisitPointers(Object** start, Object** end) {
handle_count_ += end - start;
}
......
......@@ -191,6 +191,7 @@ MaybeHandle<Object> Runtime::CreateArrayLiteralBoilerplate(
isolate->factory()->CopyFixedArray(fixed_array_values);
copied_elements_values = fixed_array_values_copy;
for (int i = 0; i < fixed_array_values->length(); i++) {
HandleScope scope(isolate);
if (fixed_array_values->get(i)->IsFixedArray()) {
// The value contains the constant_properties of a
// simple object or array literal.
......
......@@ -1499,6 +1499,7 @@ struct Tests : Rep {
void Union3() {
// Monotonicity: T1->Is(T2) or T1->Is(T3) implies T1->Is(Union(T2, T3))
for (TypeIterator it1 = T.types.begin(); it1 != T.types.end(); ++it1) {
HandleScope scope(isolate);
for (TypeIterator it2 = T.types.begin(); it2 != T.types.end(); ++it2) {
for (TypeIterator it3 = it2; it3 != T.types.end(); ++it3) {
TypeHandle type1 = *it1;
......@@ -1751,6 +1752,7 @@ struct Tests : Rep {
// Monotonicity: T1->Is(T2) and T1->Is(T3) implies T1->Is(Intersect(T2, T3))
for (TypeIterator it1 = T.types.begin(); it1 != T.types.end(); ++it1) {
HandleScope scope(isolate);
for (TypeIterator it2 = T.types.begin(); it2 != T.types.end(); ++it2) {
for (TypeIterator it3 = T.types.begin(); it3 != T.types.end(); ++it3) {
TypeHandle type1 = *it1;
......
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --check_handle_count
var ones = eval("[" + Array(12 * 1024).join("1,") + 1 + "]")
var sum = 0;
for (var i = 0; i < ones.length; i++) {
sum += ones[i];
}
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment