Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Reland of [api] Clean up scopes and precheck instantiations cache (pa…
Browse files Browse the repository at this point in the history 
…tchset #1 id:1 of https://codereview.chromium.org/2216903003/ )

Reason for revert:
asan failures are caused by a flaky stack-verflow (see https://codereview.chromium.org/2218033002 for a fix).

Original issue's description:
> Revert of [api] Clean up scopes and precheck instantiations cache (patchset #3 id:40001 of https://codereview.chromium.org/2206773003/ )
>
> Reason for revert:
> [Sheriff] Leads to mac asan failures:
> https://build.chromium.org/p/client.v8/builders/V8%20Mac64%20ASAN/builds/7835
>
> Original issue's description:
> > [api] Clean up scopes and precheck instantiations cache
> >
> > Make sure all the scopes used in api-natives.cc have inlineable constructors
> > and destructors. Additionally directly precheck the instantiations cache before
> > trying to enter the InvokeScope which sets the save_context.
> >
> > BUG=chromium:630217
> >
> > Committed: https://crrev.com/a2496b942cad524f0f3144b107936eaa9a7c9fd5
> > Cr-Commit-Position: refs/heads/master@{#38346}
>
> [email protected],[email protected]
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=chromium:630217
>
> Committed: https://crrev.com/e1b5cb43a9b90546ff5d6cea89ba17c485e842fb
> Cr-Commit-Position: refs/heads/master@{#38356}

[email protected],[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:630217

Review-Url: https://codereview.chromium.org/2217353002
Cr-Commit-Position: refs/heads/master@{#38401}
  • Loading branch information
@camillobruni
camillobruni authored and Commit bot committed Aug 5, 2016
1 parent 4943f72 commit 56c8814
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 48 deletions.
85 changes: 57 additions & 28 deletions src/api-natives.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,42 @@ namespace {

class InvokeScope {
public:
explicit InvokeScope(Isolate* isolate)
: isolate_(isolate), save_context_(isolate) {}
explicit InvokeScope(Isolate* isolate) : save_context_(isolate) {}
~InvokeScope() {
bool has_exception = isolate_->has_pending_exception();
Isolate* isolate = save_context_.isolate();
bool has_exception = isolate->has_pending_exception();
if (has_exception) {
isolate_->ReportPendingMessages();
isolate->ReportPendingMessages();
} else {
isolate_->clear_pending_message();
isolate->clear_pending_message();
}
}

private:
Isolate* isolate_;
SaveContext save_context_;
};

MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
Handle<ObjectTemplateInfo> data,
Handle<JSReceiver> new_target,
bool is_hidden_prototype);
enum class CacheCheck { kCheck, kSkip };

MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
Handle<FunctionTemplateInfo> data,
Handle<Name> name = Handle<Name>());
MaybeHandle<JSObject> InstantiateObject(
Isolate* isolate, Handle<ObjectTemplateInfo> data,
Handle<JSReceiver> new_target, CacheCheck cache_check = CacheCheck::kCheck,
bool is_hidden_prototype = false);

MaybeHandle<JSFunction> InstantiateFunction(
Isolate* isolate, Handle<FunctionTemplateInfo> data,
CacheCheck cache_check = CacheCheck::kCheck,
Handle<Name> name = Handle<Name>());

MaybeHandle<Object> Instantiate(Isolate* isolate, Handle<Object> data,
Handle<Name> name = Handle<Name>()) {
if (data->IsFunctionTemplateInfo()) {
return InstantiateFunction(isolate,
Handle<FunctionTemplateInfo>::cast(data), name);
Handle<FunctionTemplateInfo>::cast(data),
CacheCheck::kCheck, name);
} else if (data->IsObjectTemplateInfo()) {
return InstantiateObject(isolate, Handle<ObjectTemplateInfo>::cast(data),
Handle<JSReceiver>(), false);
Handle<JSReceiver>());
} else {
return data;
}
Expand Down Expand Up @@ -337,9 +339,17 @@ bool IsSimpleInstantiation(Isolate* isolate, ObjectTemplateInfo* info,
return fun->context()->native_context() == isolate->raw_native_context();
}

MaybeHandle<JSObject> InstantiateObjectWithInvokeScope(
Isolate* isolate, Handle<ObjectTemplateInfo> info,
Handle<JSReceiver> new_target) {
InvokeScope invoke_scope(isolate);
return InstantiateObject(isolate, info, new_target, CacheCheck::kSkip);
}

MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
Handle<ObjectTemplateInfo> info,
Handle<JSReceiver> new_target,
CacheCheck cache_check,
bool is_hidden_prototype) {
Handle<JSFunction> constructor;
int serial_number = Smi::cast(info->serial_number())->value();
Expand All @@ -353,7 +363,7 @@ MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
}
// Fast path.
Handle<JSObject> result;
if (serial_number) {
if (serial_number && cache_check == CacheCheck::kCheck) {
if (ProbeInstantiationsCache(isolate, serial_number).ToHandle(&result)) {
return isolate->factory()->CopyJSObject(result);
}
Expand Down Expand Up @@ -387,7 +397,6 @@ MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
if (info->immutable_proto()) {
JSObject::SetImmutableProto(object);
}
// TODO(dcarney): is this necessary?
JSObject::MigrateSlowToFast(result, 0, "ApiNatives::InstantiateObject");

if (serial_number) {
Expand All @@ -397,12 +406,18 @@ MaybeHandle<JSObject> InstantiateObject(Isolate* isolate,
return result;
}

MaybeHandle<JSFunction> InstantiateFunctionWithInvokeScope(
Isolate* isolate, Handle<FunctionTemplateInfo> info) {
InvokeScope invoke_scope(isolate);
return InstantiateFunction(isolate, info, CacheCheck::kSkip);
}

MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
Handle<FunctionTemplateInfo> data,
CacheCheck cache_check,
Handle<Name> name) {
int serial_number = Smi::cast(data->serial_number())->value();
if (serial_number) {
if (serial_number && cache_check == CacheCheck::kCheck) {
Handle<JSObject> result;
if (ProbeInstantiationsCache(isolate, serial_number).ToHandle(&result)) {
return Handle<JSFunction>::cast(result);
Expand All @@ -419,7 +434,8 @@ MaybeHandle<JSFunction> InstantiateFunction(Isolate* isolate,
InstantiateObject(
isolate,
handle(ObjectTemplateInfo::cast(prototype_templ), isolate),
Handle<JSReceiver>(), data->hidden_prototype()),
Handle<JSReceiver>(), CacheCheck::kCheck,
data->hidden_prototype()),
JSFunction);
}
Object* parent = data->parent_template();
Expand Down Expand Up @@ -488,19 +504,32 @@ void AddPropertyToPropertyList(Isolate* isolate, Handle<TemplateInfo> templ,

} // namespace


MaybeHandle<JSFunction> ApiNatives::InstantiateFunction(
Handle<FunctionTemplateInfo> data) {
Isolate* isolate = data->GetIsolate();
InvokeScope invoke_scope(isolate);
return ::v8::internal::InstantiateFunction(isolate, data);
Handle<FunctionTemplateInfo> info) {
Isolate* isolate = info->GetIsolate();
int serial_number = Smi::cast(info->serial_number())->value();
if (serial_number) {
Handle<JSObject> result;
if (ProbeInstantiationsCache(isolate, serial_number).ToHandle(&result)) {
return Handle<JSFunction>::cast(result);
}
}
return InstantiateFunctionWithInvokeScope(isolate, info);
}

MaybeHandle<JSObject> ApiNatives::InstantiateObject(
Handle<ObjectTemplateInfo> data, Handle<JSReceiver> new_target) {
Isolate* isolate = data->GetIsolate();
InvokeScope invoke_scope(isolate);
return ::v8::internal::InstantiateObject(isolate, data, new_target, false);
Handle<ObjectTemplateInfo> info, Handle<JSReceiver> new_target) {
Isolate* isolate = info->GetIsolate();
int serial_number = Smi::cast(info->serial_number())->value();
if (serial_number && !new_target.is_null() &&
IsSimpleInstantiation(isolate, *info, *new_target)) {
// Fast path.
Handle<JSObject> result;
if (ProbeInstantiationsCache(isolate, serial_number).ToHandle(&result)) {
return isolate->factory()->CopyJSObject(result);
}
}
return InstantiateObjectWithInvokeScope(isolate, info, new_target);
}

MaybeHandle<JSObject> ApiNatives::InstantiateRemoteObject(
Expand Down
14 changes: 14 additions & 0 deletions src/isolate-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ Isolate::ExceptionScope::~ExceptionScope() {
isolate_->set_pending_exception(*pending_exception_);
}

SaveContext::SaveContext(Isolate* isolate)
: isolate_(isolate), prev_(isolate->save_context()) {
if (isolate->context() != NULL) {
context_ = Handle<Context>(isolate->context());
}
isolate->set_save_context(this);
c_entry_fp_ = isolate->c_entry_fp(isolate->thread_local_top());
}

SaveContext::~SaveContext() {
isolate_->set_context(context_.is_null() ? NULL : *context_);
isolate_->set_save_context(prev_);
}

#define NATIVE_CONTEXT_FIELD_ACCESSOR(index, type, name) \
Handle<type> Isolate::name() { \
return Handle<type>(raw_native_context()->name(), this); \
Expand Down
18 changes: 0 additions & 18 deletions src/isolate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3183,24 +3183,6 @@ bool StackLimitCheck::JsHasOverflowed(uintptr_t gap) const {
return GetCurrentStackPosition() - gap < stack_guard->real_climit();
}


SaveContext::SaveContext(Isolate* isolate)
: isolate_(isolate), prev_(isolate->save_context()) {
if (isolate->context() != NULL) {
context_ = Handle<Context>(isolate->context());
}
isolate->set_save_context(this);

c_entry_fp_ = isolate->c_entry_fp(isolate->thread_local_top());
}


SaveContext::~SaveContext() {
isolate_->set_context(context_.is_null() ? NULL : *context_);
isolate_->set_save_context(prev_);
}


#ifdef DEBUG
AssertNoContextChange::AssertNoContextChange(Isolate* isolate)
: isolate_(isolate), context_(isolate->context(), isolate) {}
Expand Down
6 changes: 4 additions & 2 deletions src/isolate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1473,8 +1473,8 @@ class PromiseOnStack {
// versions of GCC. See V8 issue 122 for details.
class SaveContext BASE_EMBEDDED {
public:
explicit SaveContext(Isolate* isolate);
~SaveContext();
explicit inline SaveContext(Isolate* isolate);
inline ~SaveContext();

Handle<Context> context() { return context_; }
SaveContext* prev() { return prev_; }
Expand All @@ -1484,6 +1484,8 @@ class SaveContext BASE_EMBEDDED {
return (c_entry_fp_ == 0) || (c_entry_fp_ > frame->sp());
}

Isolate* isolate() { return isolate_; }

private:
Isolate* isolate_;
Handle<Context> context_;
Expand Down

0 comments on commit 56c8814

Please sign in to comment.