[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953

Review-Url: https://codereview.chromium.org/2328593002
Cr-Commit-Position: refs/heads/master@{#39531}

src/ast/ast-value-factory.cc

@@ -237,12 +237,7 @@ AstRawString* AstValueFactory::GetTwoByteStringInternal(
 
 
 const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
-  // For the FlatContent to stay valid, we shouldn't do any heap
-  // allocation. Make sure we won't try to internalize the string in GetString.
   AstRawString* result = NULL;
-  Isolate* saved_isolate = isolate_;
-  isolate_ = NULL;
-  {
   DisallowHeapAllocation no_gc;
   String::FlatContent content = literal->GetFlatContent();
   if (content.IsOneByte()) {
@@ -251,15 +246,6 @@ const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
     DCHECK(content.IsTwoByte());
     result = GetTwoByteStringInternal(content.ToUC16Vector());
   }
-  }
-  isolate_ = saved_isolate;
-  if (strings_ != nullptr && isolate_) {
-    // Only the string we are creating is uninternalized at this point.
-    DCHECK_EQ(result, strings_);
-    DCHECK_NULL(strings_->next());
-    result->Internalize(isolate_);
-    ResetStrings();
-  }
   return result;
 }
 
@@ -308,13 +294,6 @@ const AstRawString* AstValueFactory::ConcatStrings(const AstRawString* left,
 }
 
 void AstValueFactory::Internalize(Isolate* isolate) {
-  if (isolate_) {
-    DCHECK_NULL(strings_);
-    DCHECK_NULL(values_);
-    // Everything is already internalized.
-    return;
-  }
-
   // Strings need to be internalized before values, because values refer to
   // strings.
   for (AstString* current = strings_; current != nullptr;) {
@@ -327,7 +306,6 @@ void AstValueFactory::Internalize(Isolate* isolate) {
     current->Internalize(isolate);
     current = next;
   }
-  isolate_ = isolate;
   ResetStrings();
   values_ = nullptr;
 }

src/ast/ast-value-factory.h

@@ -329,7 +329,6 @@ class AstValueFactory {
         values_(nullptr),
         strings_end_(&strings_),
         zone_(zone),
-        isolate_(NULL),
         hash_seed_(hash_seed) {
     ResetStrings();
 #define F(name, str) name##_string_ = NULL;
@@ -359,9 +358,6 @@ class AstValueFactory {
                                     const AstRawString* right);
 
   void Internalize(Isolate* isolate);
-  bool IsInternalized() {
-    return isolate_ != NULL;
-  }
 
 #define F(name, str)                                                    \
   const AstRawString* name##_string() {                                 \
@@ -389,21 +385,13 @@ class AstValueFactory {
 
  private:
   AstValue* AddValue(AstValue* value) {
-    if (isolate_) {
-      value->Internalize(isolate_);
-    } else {
     value->set_next(values_);
     values_ = value;
-    }
     return value;
   }
   AstString* AddString(AstString* string) {
-    if (isolate_) {
-      string->Internalize(isolate_);
-    } else {
     *strings_end_ = string;
     strings_end_ = string->next_location();
-    }
     return string;
   }
   void ResetStrings() {
@@ -427,7 +415,6 @@ class AstValueFactory {
   AstString* strings_;
   AstString** strings_end_;
   Zone* zone_;
-  Isolate* isolate_;
 
   uint32_t hash_seed_;
 

src/parsing/parser.cc

@@ -4167,7 +4167,7 @@ void Parser::HandleSourceURLComments(Isolate* isolate, Handle<Script> script) {
 
 
 void Parser::Internalize(Isolate* isolate, Handle<Script> script, bool error) {
-  // Internalize strings.
+  // Internalize strings and values.
   ast_value_factory()->Internalize(isolate);
 
   // Error processing.
@@ -4234,7 +4234,6 @@ bool Parser::Parse(ParseInfo* info) {
   info->set_literal(result);
 
   Internalize(isolate, info->script(), result == NULL);
-  DCHECK(ast_value_factory()->IsInternalized());
   return (result != NULL);
 }
 
@@ -5827,13 +5826,6 @@ Statement* Parser::FinalizeForOfStatement(ForOfStatement* loop,
   return final_loop;
 }
 
-#ifdef DEBUG
-void Parser::Print(AstNode* node) {
-  ast_value_factory()->Internalize(Isolate::Current());
-  node->Print(Isolate::Current());
-}
-#endif  // DEBUG
-
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED

src/parsing/parser.h

@@ -1082,10 +1082,6 @@ class Parser : public ParserBase<Parser> {
   HistogramTimer* pre_parse_timer_;
 
   bool parsing_on_main_thread_;
-
-#ifdef DEBUG
-  void Print(AstNode* node);
-#endif  // DEBUG
 };
 
 }  // namespace internal

src/parsing/rewriter.cc

@@ -347,10 +347,13 @@ bool Rewriter::Rewrite(ParseInfo* info) {
     Variable* result = closure_scope->NewTemporary(
         info->ast_value_factory()->dot_result_string());
     // The name string must be internalized at this point.
+    info->ast_value_factory()->Internalize(info->isolate());
     DCHECK(!result->name().is_null());
     Processor processor(info->isolate(), closure_scope, result,
                         info->ast_value_factory());
     processor.Process(body);
+    // Internalize any values created during rewriting.
+    info->ast_value_factory()->Internalize(info->isolate());
     if (processor.HasStackOverflow()) return false;
 
     if (processor.result_assigned()) {

test/cctest/asmjs/test-asm-typer.cc

@@ -276,6 +276,7 @@ class AsmTyperHarnessBuilder {
  private:
   Variable* DeclareVariable(VariableName var_name) {
     auto* name_ast_string = ast_value_factory_.GetOneByteString(var_name.name_);
+    ast_value_factory_.Internalize(isolate_);
     return var_name.mode_ == DYNAMIC_GLOBAL
                ? outer_scope_->DeclareDynamicGlobal(name_ast_string,
                                                     NORMAL_VARIABLE)

test/cctest/test-parsing.cc

@@ -632,9 +632,10 @@ void TestScanRegExp(const char* re_source, const char* expected) {
   i::Zone zone(CcTest::i_isolate()->allocator());
   i::AstValueFactory ast_value_factory(&zone,
                                        CcTest::i_isolate()->heap()->HashSeed());
+  const i::AstRawString* current_symbol =
+      scanner.CurrentSymbol(&ast_value_factory);
   ast_value_factory.Internalize(CcTest::i_isolate());
-  i::Handle<i::String> val =
-      scanner.CurrentSymbol(&ast_value_factory)->string();
+  i::Handle<i::String> val = current_symbol->string();
   i::DisallowHeapAllocation no_alloc;
   i::String::FlatContent content = val->GetFlatContent();
   CHECK(content.IsOneByte());
@@ -3214,8 +3215,8 @@ TEST(SerializationOfMaybeAssignmentFlag) {
   i::Handle<i::JSFunction> f = i::Handle<i::JSFunction>::cast(o);
   i::Context* context = f->context();
   i::AstValueFactory avf(&zone, isolate->heap()->HashSeed());
-  avf.Internalize(isolate);
   const i::AstRawString* name = avf.GetOneByteString("result");
+  avf.Internalize(isolate);
   i::Handle<i::String> str = name->string();
   CHECK(str->IsInternalizedString());
   i::DeclarationScope* script_scope =
@@ -3264,6 +3265,7 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
   i::Handle<i::JSFunction> f = i::Handle<i::JSFunction>::cast(o);
   i::Context* context = f->context();
   i::AstValueFactory avf(&zone, isolate->heap()->HashSeed());
+  const i::AstRawString* name_x = avf.GetOneByteString("x");
   avf.Internalize(isolate);
 
   i::DeclarationScope* script_scope =
@@ -3272,7 +3274,6 @@ TEST(IfArgumentsArrayAccessedThenParametersMaybeAssigned) {
       isolate, &zone, context->scope_info(), script_scope, &avf,
       i::Scope::DeserializationMode::kIncludingVariables);
   CHECK(s != script_scope);
-  const i::AstRawString* name_x = avf.GetOneByteString("x");
 
   // Get result from f's function context (that is g's outer context)
   i::Variable* var_x = s->Lookup(name_x);