[v8-dev] Don't use ConsStringIterator to compute string hashes (issue 762773002 by [email protected])

Wed, 26 Nov 2014 02:16:43 -0800 

Reviewers: Yang,

Message:
ptal - the absolute worst case for hashing a constring is only a 15ms or so, but 
it's about 0 doing this

Description:
Don't use ConsStringIterator to compute string hashes

[email protected]

BUG=

Please review this at https://codereview.chromium.org/762773002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+26, -7 lines):
  M src/isolate.h
  M src/isolate.cc
  M src/objects.h
  M src/objects.cc
  M src/objects-inl.h


Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index 09ca02854f12282e81d70327209e7c0f52f1c903..6f19075ac5b9e02eb1084891028b9238078396c9 100644 
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1635,6 +1635,7 @@ Isolate::Isolate(bool enable_serializer)
       thread_manager_(NULL),
       has_installed_extensions_(false),
       string_tracker_(NULL),
+      cons_string_hashing_buffer_(nullptr),
       regexp_stack_(NULL),
       date_cache_(NULL),
       call_descriptor_data_(NULL),
@@ -1881,6 +1882,9 @@ Isolate::~Isolate() {
   delete eternal_handles_;
   eternal_handles_ = NULL;

+  delete[] cons_string_hashing_buffer_;
+  cons_string_hashing_buffer_ = nullptr;
+
   delete string_stream_debug_object_cache_;
   string_stream_debug_object_cache_ = NULL;

@@ -1996,6 +2000,7 @@ bool Isolate::Init(Deserializer* des) {
   handle_scope_implementer_ = new HandleScopeImplementer(this);
   stub_cache_ = new StubCache(this);
   materialized_object_store_ = new MaterializedObjectStore(this);
+  cons_string_hashing_buffer_ = new uint16_t[String::kMaxHashCalcLength];
   regexp_stack_ = new RegExpStack();
   regexp_stack_->isolate_ = this;
   date_cache_ = new DateCache();
Index: src/isolate.h
diff --git a/src/isolate.h b/src/isolate.h
index 96e044596d29a67bc71aa4c746a44b0c2180f1ac..0bdbd84bc583a1bd4bb4c39efdd6803ece083ba0 100644 
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -962,6 +962,8 @@ class Isolate {
     return &regexp_macro_assembler_canonicalize_;
   }
+ uint16_t* cons_string_hashing_buffer() { return cons_string_hashing_buffer_; } 
+
   RegExpStack* regexp_stack() { return regexp_stack_; }

   unibrow::Mapping<unibrow::Ecma262Canonicalize>*
@@ -1264,6 +1266,7 @@ class Isolate {
   unibrow::Mapping<unibrow::CanonicalizationRange> jsregexp_canonrange_;
   unibrow::Mapping<unibrow::Ecma262Canonicalize>
       regexp_macro_assembler_canonicalize_;
+  uint16_t* cons_string_hashing_buffer_;
   RegExpStack* regexp_stack_;
   DateCache* date_cache_;
unibrow::Mapping<unibrow::Ecma262Canonicalize> interp_canonicalize_mapping_; 
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 9172257e6d8c884e5448605c4b7ee6ded30c698c..03aea64736d818f456c748f76a31d583593dc664 100644 
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -6787,13 +6787,8 @@ uint32_t IteratingStringHasher::Hash(String* string, uint32_t seed) { 
   // Nothing to do.
   if (hasher.has_trivial_hash()) return hasher.GetHashField();
   ConsString* cons_string = String::VisitFlat(&hasher, string);
-  // The string was flat.
-  if (cons_string == NULL) return hasher.GetHashField();
-  // This is a ConsString, iterate across it.
-  ConsStringIterator iter(cons_string);
-  int offset;
-  while (NULL != (string = iter.Next(&offset))) {
-    String::VisitFlat(&hasher, string, offset);
+  if (cons_string != nullptr) {
+    hasher.VisitConsString(cons_string);
   }
   return hasher.GetHashField();
 }
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 5262ed65671afa93c941173f9124813ab82c56ab..7900ef3bcb35d2e5c405add63c43809b2c5ee27a 100644 
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9283,6 +9283,20 @@ uint32_t StringHasher::ComputeUtf8Hash(Vector<const char> chars, 
 }


+void IteratingStringHasher::VisitConsString(ConsString* cons_string) {
+  int length = cons_string->length();
+ uint16_t* buffer = cons_string->GetIsolate()->cons_string_hashing_buffer(); 
+  if (cons_string->HasOnlyOneByteChars()) {
+    uint8_t* one_byte_buffer = reinterpret_cast<uint8_t*>(buffer);
+    String::WriteToFlat(cons_string, one_byte_buffer, 0, length);
+    AddCharacters(one_byte_buffer, length);
+  } else {
+    String::WriteToFlat(cons_string, buffer, 0, length);
+    AddCharacters(buffer, length);
+  }
+}
+
+
 void String::PrintOn(FILE* file) {
   int length = this->length();
   for (int i = 0; i < length; i++) {
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index 664a5064c706697b50a81f4c6994372e9bec023f..746f88d1c4c0c10083e63263a005f79ec4378b9d 100644 
--- a/src/objects.h
+++ b/src/objects.h
@@ -850,6 +850,7 @@ class AccessorPair;
 class AllocationSite;
 class AllocationSiteCreationContext;
 class AllocationSiteUsageContext;
+class ConsString;
 class DictionaryElementsAccessor;
 class ElementsAccessor;
 class FixedArrayBase;
@@ -8524,6 +8525,7 @@ class IteratingStringHasher : public StringHasher {
  private:
   inline IteratingStringHasher(int len, uint32_t seed)
       : StringHasher(len, seed) {}
+  void VisitConsString(ConsString* cons_string);
   DISALLOW_COPY_AND_ASSIGN(IteratingStringHasher);
 };



--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to