Extension state made per-siolate in genesis

BUG=http://code.google.com/p/v8/issues/detail?id=1821

Review URL: http://codereview.chromium.org/8536042

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10001 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -486,7 +486,7 @@ RegisteredExtension* RegisteredExtension::first_extension_ = NULL;
 
 
 RegisteredExtension::RegisteredExtension(Extension* extension)
-    : extension_(extension), state_(UNVISITED) { }
+    : extension_(extension) { }
 
 
 void RegisteredExtension::Register(RegisteredExtension* that) {

src/api.h

@@ -137,10 +137,6 @@ class ApiFunction {
 };
 
 
-enum ExtensionTraversalState {
-  UNVISITED, VISITED, INSTALLED
-};
-
 
 class RegisteredExtension {
  public:
@@ -149,14 +145,11 @@ class RegisteredExtension {
   Extension* extension() { return extension_; }
   RegisteredExtension* next() { return next_; }
   RegisteredExtension* next_auto() { return next_auto_; }
-  ExtensionTraversalState state() { return state_; }
-  void set_state(ExtensionTraversalState value) { state_ = value; }
   static RegisteredExtension* first_extension() { return first_extension_; }
  private:
   Extension* extension_;
   RegisteredExtension* next_;
   RegisteredExtension* next_auto_;
-  ExtensionTraversalState state_;
   static RegisteredExtension* first_extension_;
 };
 

src/bootstrapper.cc

@@ -211,12 +211,31 @@ class Genesis BASE_EMBEDDED {
   void InstallBuiltinFunctionIds();
   void InstallJSFunctionResultCaches();
   void InitializeNormalizedMapCaches();
+  
+  enum ExtensionTraversalState {
+    UNVISITED, VISITED, INSTALLED
+  };
+
+  class ExtensionStates {
+    DISALLOW_COPY_AND_ASSIGN(ExtensionStates);
+  public:
+    ExtensionStates();
+    ExtensionTraversalState get_state(RegisteredExtension* extension);
+    void set_state(RegisteredExtension* extension,
+                   ExtensionTraversalState state);
+  private:
+    Allocator allocator_;
+    HashMap map_;
+  };
+
   // Used both for deserialized and from-scratch contexts to add the extensions
   // provided.
   static bool InstallExtensions(Handle<Context> global_context,
                                 v8::ExtensionConfiguration* extensions);
-  static bool InstallExtension(const char* name);
-  static bool InstallExtension(v8::RegisteredExtension* current);
+  static bool InstallExtension(const char* name,
+                               ExtensionStates* extension_states);
+  static bool InstallExtension(v8::RegisteredExtension* current,
+                               ExtensionStates* extension_states);
   static void InstallSpecialObjects(Handle<Context> global_context);
   bool InstallJSBuiltins(Handle<JSBuiltinsObject> builtins);
   bool ConfigureApiObject(Handle<JSObject> object,
@@ -1935,6 +1954,34 @@ void Genesis::InstallSpecialObjects(Handle<Context> global_context) {
 #endif
 }
 
+static uint32_t Hash(RegisteredExtension* extension) {
+  return v8::internal::ComputePointerHash(extension);
+}
+
+static bool MatchRegisteredExtensions(void* key1, void* key2) {
+  return key1 == key2;
+}
+
+Genesis::ExtensionStates::ExtensionStates()
+  : allocator_(),
+    map_(MatchRegisteredExtensions, &allocator_, 8)
+  {}
+
+Genesis::ExtensionTraversalState Genesis::ExtensionStates::get_state(
+    RegisteredExtension* extension) {
+  i::HashMap::Entry* entry = map_.Lookup(extension, Hash(extension), false);
+  if (entry == NULL) {
+    return UNVISITED;
+  }
+  return static_cast<ExtensionTraversalState>(
+      reinterpret_cast<intptr_t>(entry->value));
+}
+
+void Genesis::ExtensionStates::set_state(RegisteredExtension* extension,
+                                         ExtensionTraversalState state) {
+  map_.Lookup(extension, Hash(extension), true)->value =
+      reinterpret_cast<void*>(static_cast<intptr_t>(state));
+}
 
 bool Genesis::InstallExtensions(Handle<Context> global_context,
                                 v8::ExtensionConfiguration* extensions) {
@@ -1942,29 +1989,26 @@ bool Genesis::InstallExtensions(Handle<Context> global_context,
   //                 effort. (The external API reads 'ignore'-- does that mean
   //                 we can break the interface?)
 
-  // Clear coloring of extension list
-  v8::RegisteredExtension* current = v8::RegisteredExtension::first_extension();
-  while (current != NULL) {
-    current->set_state(v8::UNVISITED);
-    current = current->next();
-  }
+  ExtensionStates extension_states; // All extensions have state UNVISITED.
   // Install auto extensions.
-  current = v8::RegisteredExtension::first_extension();
+  v8::RegisteredExtension* current = v8::RegisteredExtension::first_extension();
   while (current != NULL) {
     if (current->extension()->auto_enable())
-      InstallExtension(current);
+      InstallExtension(current, &extension_states);
     current = current->next();
   }
 
-  if (FLAG_expose_gc) InstallExtension("v8/gc");
-  if (FLAG_expose_externalize_string) InstallExtension("v8/externalize");
+  if (FLAG_expose_gc) InstallExtension("v8/gc", &extension_states);
+  if (FLAG_expose_externalize_string) {
+    InstallExtension("v8/externalize", &extension_states);
+  }
 
   if (extensions == NULL) return true;
   // Install required extensions
   int count = v8::ImplementationUtilities::GetNameCount(extensions);
   const char** names = v8::ImplementationUtilities::GetNames(extensions);
   for (int i = 0; i < count; i++) {
-    if (!InstallExtension(names[i]))
+    if (!InstallExtension(names[i], &extension_states))
       return false;
   }
 
@@ -1974,7 +2018,8 @@ bool Genesis::InstallExtensions(Handle<Context> global_context,
 
 // Installs a named extension.  This methods is unoptimized and does
 // not scale well if we want to support a large number of extensions.
-bool Genesis::InstallExtension(const char* name) {
+bool Genesis::InstallExtension(const char* name,
+                               ExtensionStates* extension_states) {
   v8::RegisteredExtension* current = v8::RegisteredExtension::first_extension();
   // Loop until we find the relevant extension
   while (current != NULL) {
@@ -1987,27 +2032,29 @@ bool Genesis::InstallExtension(const char* name) {
         "v8::Context::New()", "Cannot find required extension");
     return false;
   }
-  return InstallExtension(current);
+  return InstallExtension(current, extension_states);
 }
 
 
-bool Genesis::InstallExtension(v8::RegisteredExtension* current) {
+bool Genesis::InstallExtension(v8::RegisteredExtension* current, 
+                               ExtensionStates* extension_states) {
   HandleScope scope;
 
-  if (current->state() == v8::INSTALLED) return true;
+  if (extension_states->get_state(current) == INSTALLED) return true;
   // The current node has already been visited so there must be a
   // cycle in the dependency graph; fail.
-  if (current->state() == v8::VISITED) {
+  if (extension_states->get_state(current) == VISITED) {
     v8::Utils::ReportApiFailure(
         "v8::Context::New()", "Circular extension dependency");
     return false;
   }
-  ASSERT(current->state() == v8::UNVISITED);
-  current->set_state(v8::VISITED);
+  ASSERT(extension_states->get_state(current) == UNVISITED);
+  extension_states->set_state(current, VISITED);
   v8::Extension* extension = current->extension();
   // Install the extension's dependencies
   for (int i = 0; i < extension->dependency_count(); i++) {
-    if (!InstallExtension(extension->dependencies()[i])) return false;
+    if (!InstallExtension(extension->dependencies()[i], extension_states))
+      return false;
   }
   Isolate* isolate = Isolate::Current();
   Handle<String> source_code =
@@ -2029,7 +2076,8 @@ bool Genesis::InstallExtension(v8::RegisteredExtension* current) {
                    current->extension()->name());
     isolate->clear_pending_exception();
   }
-  current->set_state(v8::INSTALLED);
+  extension_states->set_state(current, INSTALLED);
+  isolate->NotifyExtensionInstalled();
   return result;
 }
 

src/isolate.cc

@@ -1434,6 +1434,7 @@ Isolate::Isolate()
       context_switcher_(NULL),
       thread_manager_(NULL),
       fp_stubs_generated_(false),
+      has_installed_extensions_(false),
       string_tracker_(NULL),
       regexp_stack_(NULL),
       embedder_data_(NULL) {

src/isolate.h

@@ -895,6 +895,12 @@ class Isolate {
 
   Builtins* builtins() { return &builtins_; }
 
+  void NotifyExtensionInstalled() {
+    has_installed_extensions_ = true;
+  }
+
+  bool has_installed_extensions() { return has_installed_extensions_; }
+
   unibrow::Mapping<unibrow::Ecma262Canonicalize>*
       regexp_macro_assembler_canonicalize() {
     return &regexp_macro_assembler_canonicalize_;
@@ -1156,6 +1162,7 @@ class Isolate {
   bool fp_stubs_generated_;
   StaticResource<SafeStringInputBuffer> compiler_safe_string_input_buffer_;
   Builtins builtins_;
+  bool has_installed_extensions_;
   StringTracker* string_tracker_;
   unibrow::Mapping<unibrow::Ecma262UnCanonicalize> jsregexp_uncanonicalize_;
   unibrow::Mapping<unibrow::CanonicalizationRange> jsregexp_canonrange_;

src/serialize.cc

@@ -1132,11 +1132,8 @@ void StartupSerializer::SerializeStrongReferences() {
   CHECK(isolate->handle_scope_implementer()->blocks()->is_empty());
   CHECK_EQ(0, isolate->global_handles()->NumberOfWeakHandles());
   // We don't support serializing installed extensions.
-  for (RegisteredExtension* ext = v8::RegisteredExtension::first_extension();
-       ext != NULL;
-       ext = ext->next()) {
-    CHECK_NE(v8::INSTALLED, ext->state());
-  }
+  CHECK(!isolate->has_installed_extensions());
+
   HEAP->IterateStrongRoots(this, VISIT_ONLY_STRONG);
 }
 

test/cctest/test-lockers.cc

@@ -639,3 +639,64 @@ TEST(Regress1433) {
     isolate->Dispose();
   }
 }
+
+
+static const char* kSimpleExtensionSource =
+  "(function Foo() {"
+  "  return 4;"
+  "})() ";
+
+class IsolateGenesisThread : public JoinableThread {
+ public:
+  IsolateGenesisThread(int count, const char* extension_names[])
+    : JoinableThread("IsolateGenesisThread"),
+      count_(count),
+      extension_names_(extension_names)
+  {}
+
+  virtual void Run() {
+    v8::Isolate* isolate = v8::Isolate::New();
+    {
+      v8::Isolate::Scope isolate_scope(isolate);
+      CHECK(!i::Isolate::Current()->has_installed_extensions());
+      v8::ExtensionConfiguration extensions(count_, extension_names_);
+      v8::Persistent<v8::Context> context = v8::Context::New(&extensions);
+      CHECK(i::Isolate::Current()->has_installed_extensions());
+      context.Dispose();
+    }
+    isolate->Dispose();
+  }
+ private:
+  int count_;
+  const char** extension_names_;
+};
+
+// Test installing extensions in separate isolates concurrently.
+// http://code.google.com/p/v8/issues/detail?id=1821
+TEST(ExtensionsRegistration) {
+  const int kNThreads = 40;
+  v8::RegisterExtension(new v8::Extension("test0",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test1",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test2",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test3",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test4",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test5",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test6",
+                                          kSimpleExtensionSource));
+  v8::RegisterExtension(new v8::Extension("test7",
+                                          kSimpleExtensionSource));
+  const char* extension_names[] = { "test0", "test1",
+                                    "test2", "test3", "test4",
+                                    "test5", "test6", "test7" };
+  i::List<JoinableThread*> threads(kNThreads);
+  for (int i = 0; i < kNThreads; i++) {
+    threads.Add(new IsolateGenesisThread(8, extension_names));
+  }
+  StartJoinAndDeleteThreads(threads);
+}