From 94a08565d79a13056b9d964428798dac564905bc Mon Sep 17 00:00:00 2001
From: "marja@chromium.org" <marja@chromium.org>
Date: Tue, 4 Nov 2014 14:15:57 +0000
Subject: [PATCH] Parser & internalizing: efficiency fixes.

1) In ParseLazy, we're already using the heap, so we can also tell
AstValueFactory to internalize immediately. This is more efficient.

2) No need to collect the values in values_ and strings_ if they're already
internalized.

3) No need to collect AstValues which are strings in values_ since they don't
need to be internalized (the underlying strings will be internalized
separately).

BUG=429168
LOG=N
R=rossberg@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#25109}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25109 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/ast-value-factory.cc | 27 +++++++++++++++++----------
 src/parser.cc            |  5 +++++
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/ast-value-factory.cc b/src/ast-value-factory.cc
index 895ce39..90c472d 100644
--- a/src/ast-value-factory.cc
+++ b/src/ast-value-factory.cc
@@ -241,12 +241,11 @@ const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
 
 const AstConsString* AstValueFactory::NewConsString(
     const AstString* left, const AstString* right) {
-  // This Vector will be valid as long as the Collector is alive (meaning that
-  // the AstRawString will not be moved).
   AstConsString* new_string = new (zone_) AstConsString(left, right);
-  strings_.Add(new_string);
   if (isolate_) {
     new_string->Internalize(isolate_);
+  } else {
+    strings_.Add(new_string);
   }
   return new_string;
 }
@@ -273,9 +272,12 @@ const AstValue* AstValueFactory::NewString(const AstRawString* string) {
   AstValue* value = new (zone_) AstValue(string);
   DCHECK(string != NULL);
   if (isolate_) {
-    value->Internalize(isolate_);
+    // If we're creating immediately-internalized AstValues, the underlying
+    // strings must already be internalized at this point.
+    DCHECK(!string->string_.is_null());
   }
-  values_.Add(value);
+  // These AstValues don't need to be added to values_, since the AstRawStrings
+  // will be insternalized separately.
   return value;
 }
 
@@ -284,8 +286,9 @@ const AstValue* AstValueFactory::NewSymbol(const char* name) {
   AstValue* value = new (zone_) AstValue(name);
   if (isolate_) {
     value->Internalize(isolate_);
+  } else {
+    values_.Add(value);
   }
-  values_.Add(value);
   return value;
 }
 
@@ -294,8 +297,9 @@ const AstValue* AstValueFactory::NewNumber(double number) {
   AstValue* value = new (zone_) AstValue(number);
   if (isolate_) {
     value->Internalize(isolate_);
+  } else {
+    values_.Add(value);
   }
-  values_.Add(value);
   return value;
 }
 
@@ -305,8 +309,9 @@ const AstValue* AstValueFactory::NewSmi(int number) {
       new (zone_) AstValue(AstValue::SMI, number);
   if (isolate_) {
     value->Internalize(isolate_);
+  } else {
+    values_.Add(value);
   }
-  values_.Add(value);
   return value;
 }
 
@@ -316,8 +321,9 @@ const AstValue* AstValueFactory::NewSmi(int number) {
     value = new (zone_) AstValue(initializer);    \
     if (isolate_) {                               \
       value->Internalize(isolate_);               \
+    } else {                                      \
+      values_.Add(value);                         \
     }                                             \
-    values_.Add(value);                           \
   }                                               \
   return value;
 
@@ -364,9 +370,10 @@ const AstRawString* AstValueFactory::GetString(
     AstRawString* new_string = new (zone_) AstRawString(
         is_one_byte, Vector<const byte>(new_literal_bytes, length), hash);
     entry->key = new_string;
-    strings_.Add(new_string);
     if (isolate_) {
       new_string->Internalize(isolate_);
+    } else {
+      strings_.Add(new_string);
     }
     entry->value = reinterpret_cast<void*>(1);
   }
diff --git a/src/parser.cc b/src/parser.cc
index c5bf0d9..7a88c52 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -935,6 +935,11 @@ FunctionLiteral* Parser::ParseLazy() {
   }
   Handle<SharedFunctionInfo> shared_info = info()->shared_info();
 
+  // Lazy parsing is only done during runtime, when we're already using the
+  // heap. So make the AstValueFactory also internalize all values when it
+  // creates them (this is more efficient).
+  ast_value_factory()->Internalize(isolate());
+
   // Initialize parser state.
   source = String::Flatten(source);
   FunctionLiteral* result;
-- 
2.7.4