Skip to content

Commit

Permalink
Reland 'Additional HandleScopes to limit Handle consumption.'
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
ohodson authored and Commit bot committed Jun 26, 2015
1 parent 572cac6 commit af4c4b0
Show file tree
Hide file tree
Showing 8 changed files with 1,270 additions and 4 deletions.
4 changes: 3 additions & 1 deletion src/ast.cc
Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion src/handles-inl.h
Expand Up @@ -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
}


Expand Down
5 changes: 5 additions & 0 deletions src/handles.h
Expand Up @@ -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&);
Expand Down
6 changes: 4 additions & 2 deletions src/heap/heap.cc
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/runtime/runtime-literals.cc
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions test/cctest/test-types.cc
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions test/mjsunit/handle-count-ast.js
@@ -0,0 +1,12 @@
// 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];
}
1,230 changes: 1,230 additions & 0 deletions test/mjsunit/handle-count-runtime-literals.js

Large diffs are not rendered by default.

0 comments on commit af4c4b0

Please sign in to comment.