Optimize the equality check case of ICCompare stubs.

src/code-stubs.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 83 matching lines @@
 int CodeStub::GetCodeKind() {
   return Code::STUB;
 }
 
 
 Handle<Code> CodeStub::GetCode() {
   Isolate* isolate = Isolate::Current();
   Factory* factory = isolate->factory();
   Heap* heap = isolate->heap();
   Code* code;
-  if (!FindCodeInCache(&code)) {
+  if (UseSpecialCache()
+      ? FindCodeInSpecialCache(&code)
+      : FindCodeInCache(&code)) {
+    CHECK(IsPregenerated() == code->is_pregenerated());

[Kevin Millikin (Chromium), 2011/11/11 10:12:34]

Maybe I'm missing a good reason for this, but please change this to an ASSERT.

[Rico, 2011/11/15 10:12:26]

Done (checked with Erik that we can actally make this an ASSERT again (it was
introduced in revision 9482 to catch a bug)

+    return Handle<Code>(code);
+  }
+
+  {
     HandleScope scope(isolate);
 
     // Generate the new code.
     MacroAssembler masm(isolate, NULL, 256);
     GenerateCode(&masm);
 
     // Create the code object.
     CodeDesc desc;
     masm.GetCode(&desc);
 
     // Copy the generated code into a heap object.
     Code::Flags flags = Code::ComputeFlags(
         static_cast<Code::Kind>(GetCodeKind()),
         GetICState());
     Handle<Code> new_object = factory->NewCode(
         desc, flags, masm.CodeObject(), NeedsImmovableCode());
     RecordCodeGeneration(*new_object, &masm);
     FinishCode(*new_object);
 
     // Update the dictionary and the root in Heap.
-    Handle<NumberDictionary> dict =
-        factory->DictionaryAtNumberPut(
-            Handle<NumberDictionary>(heap->code_stubs()),
-            GetKey(),
-            new_object);
-    heap->public_set_code_stubs(*dict);
+
+    if (UseSpecialCache()) {
+      CHECK(AddToSpecialCache(new_object));

[Kevin Millikin (Chromium), 2011/11/11 10:12:34]

This function can only return true, so get rid of the CHECK and make the return
type void.

[Rico, 2011/11/15 10:12:26]

Done.

+    } else {
+      Handle<NumberDictionary> dict =
+          factory->DictionaryAtNumberPut(
+              Handle<NumberDictionary>(heap->code_stubs()),
+              GetKey(),
+              new_object);
+      heap->public_set_code_stubs(*dict);
+    }
     code = *new_object;
-    Activate(code);
-  } else {
-    CHECK(IsPregenerated() == code->is_pregenerated());
   }
 
+  Activate(code);
   ASSERT(!NeedsImmovableCode() || heap->lo_space()->Contains(code));
   return Handle<Code>(code, isolate);
 }
 
 
 const char* CodeStub::MajorName(CodeStub::Major major_key,
                                 bool allow_unknown_keys) {
   switch (major_key) {
 #define DEF_CASE(name) case name: return #name "Stub";
     CODE_STUB_LIST(DEF_CASE)
 #undef DEF_CASE
     default:
       if (!allow_unknown_keys) {
         UNREACHABLE();
       }
       return NULL;
   }
 }
 
 
 void CodeStub::PrintName(StringStream* stream) {
   stream->Add("%s", MajorName(MajorKey(), false));
 }
 
 
+bool ICCompareStub::AddToSpecialCache(Handle<Code> new_object) {
+  ASSERT(known_map_ != NULL);
+  Isolate* isolate = new_object->GetIsolate();

[Kevin Millikin (Chromium), 2011/11/11 10:12:34]

I still don't fully understand why we need a separate caching mechanism here
(but didn't before).  I want to talk to the GC gurus a bit more.

I'm worried that adding special caches doesn't scale.  Could we instead add just
a special symbol and use the cache in the map.  The implementation would become
something like:

void ICCompareStub::AddToSpecialCache(Handle<Code> new_object) {
  return Map::UpdateCodeCache(known_map_,
                              factory()->compare_ic_symbol(),
                              new_object);
}

[Rico, 2011/11/15 10:12:26]

I discussed with Erik and found a solution for using the code-cache (see the new
change in mark-compact.cc)

+  Factory* factory = isolate->factory();
+  Heap* heap = isolate->heap();
+  Handle<NumberDictionary> dict =
+      factory->DictionaryAtNumberPut(
+          Handle<NumberDictionary>(heap->compare_stubs_known_objects()),
+          static_cast<uint32_t>(reinterpret_cast<uintptr_t>(*known_map_)),
+          new_object);
+  heap->public_set_compare_stubs_known_objects(*dict);
+  return true;
+}
+
+
+bool ICCompareStub::FindCodeInSpecialCache(Code** code_out) {
+  ASSERT(known_map_ != NULL);
+  Heap* heap = Isolate::Current()->heap();
+  int index = heap->compare_stubs_known_objects()->FindEntry(
+      static_cast<uint32_t>(reinterpret_cast<uintptr_t>(*known_map_)));
+  if (index != NumberDictionary::kNotFound) {
+    *code_out = Code::cast(heap->compare_stubs_known_objects()->ValueAt(index));
+    UNREACHABLE();

[Kevin Millikin (Chromium), 2011/11/11 10:12:34]

Huh?

[Rico, 2011/11/15 10:12:26]

Done.

+    return true;
+  }
+  return false;
+}
+
+
 int ICCompareStub::MinorKey() {
   return OpField::encode(op_ - Token::EQ) | StateField::encode(state_);
 }
 
 
 void ICCompareStub::Generate(MacroAssembler* masm) {
   switch (state_) {
     case CompareIC::UNINITIALIZED:
       GenerateMiss(masm);
       break;
     case CompareIC::SMIS:
       GenerateSmis(masm);
       break;
     case CompareIC::HEAP_NUMBERS:
       GenerateHeapNumbers(masm);
       break;
     case CompareIC::STRINGS:
       GenerateStrings(masm);
       break;
     case CompareIC::SYMBOLS:
       GenerateSymbols(masm);
       break;
     case CompareIC::OBJECTS:
       GenerateObjects(masm);
       break;
+    case CompareIC::KNOWN_OBJECTS:
+      ASSERT(known_map_ != NULL);
+      GenerateKnownObjects(masm, known_map_);

[Kevin Millikin (Chromium), 2011/11/11 10:12:34]

No real need to pass known_map_ here.

[Rico, 2011/11/15 10:12:26]

Done.

+      break;
     default:
       UNREACHABLE();
   }
 }
 
 
 void InstanceofStub::PrintName(StringStream* stream) {
   const char* args = "";
   if (HasArgsInRegisters()) {
     args = "_REGS";
@@ skipping 196 matching lines @@
       KeyedStoreStubCompiler::GenerateStoreFastDoubleElement(masm, is_jsarray_);
     } else {
       UNREACHABLE();
     }
   }
   masm->bind(&fail);
   KeyedStoreIC::GenerateRuntimeSetProperty(masm, strict_mode_);
 }
 
 } }  // namespace v8::internal