Reland of [api] Clean up scopes and precheck instantiations cache (patchset #1...

Reland of [api] Clean up scopes and precheck instantiations cache (patchset #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}
>
> TBR=yangguo@chromium.org,cbruni@chromium.org
> # 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}

TBR=yangguo@chromium.org,machenbach@chromium.org
# 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}

src/api-natives.cc

@@ -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;
   }
@@ -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();
@@ -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);
     }
@@ -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) {
@@ -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);
@@ -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();
@@ -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(

src/isolate-inl.h

@@ -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); \

src/isolate.cc

@@ -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) {}

src/isolate.h

@@ -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_; }
@@ -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_;