Revision: 10216
Author: [email protected]
Date: Thu Dec 8 09:17:21 2011
Log: Optimize the equality check case of ICCompare stubs.
This includes specialcasing the generation when we know that the maps
of the two objects are the same. In addition, a new specialized
compare ic known objects cache is created.
The reason for the cache is that we need to have access to the stub
code from the roots; if we do not, the GC will collect the stub. In
this specialized case we use the map pointer as key in the cache, and
we always do a lookup before generating code. Actually hitting
something in the cache will happen very rarely, but we could
potentially overwrite an existing stub, which again will lead to the
GC collecting this old stub (even if it is referenced from other code
objects)
Review URL: http://codereview.chromium.org/8520006
http://code.google.com/p/v8/source/detail?r=10216
Modified:
/branches/bleeding_edge/src/code-stubs.cc
/branches/bleeding_edge/src/code-stubs.h
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/heap.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/ia32/ic-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/ic.h
/branches/bleeding_edge/src/mark-compact.cc
/branches/bleeding_edge/src/type-info.cc
/branches/bleeding_edge/src/type-info.h
=======================================
--- /branches/bleeding_edge/src/code-stubs.cc Fri Nov 11 05:48:14 2011
+++ /branches/bleeding_edge/src/code-stubs.cc Thu Dec 8 09:17:21 2011
@@ -101,7 +101,14 @@
Factory* factory = isolate->factory();
Heap* heap = isolate->heap();
Code* code;
- if (!FindCodeInCache(&code)) {
+ if (UseSpecialCache()
+ ? FindCodeInSpecialCache(&code)
+ : FindCodeInCache(&code)) {
+ ASSERT(IsPregenerated() == code->is_pregenerated());
+ return Handle<Code>(code);
+ }
+
+ {
HandleScope scope(isolate);
// Generate the new code.
@@ -121,19 +128,21 @@
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()) {
+ AddToSpecialCache(new_object);
+ } else {
+ // 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);
+ }
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);
}
@@ -157,6 +166,32 @@
void CodeStub::PrintName(StringStream* stream) {
stream->Add("%s", MajorName(MajorKey(), false));
}
+
+
+void ICCompareStub::AddToSpecialCache(Handle<Code> new_object) {
+ ASSERT(*known_map_ != NULL);
+ Isolate* isolate = new_object->GetIsolate();
+ Factory* factory = isolate->factory();
+ return Map::UpdateCodeCache(known_map_,
+ factory->compare_ic_symbol(),
+ new_object);
+}
+
+
+bool ICCompareStub::FindCodeInSpecialCache(Code** code_out) {
+ Isolate* isolate = known_map_->GetIsolate();
+ Factory* factory = isolate->factory();
+ Code::Flags flags = Code::ComputeFlags(
+ static_cast<Code::Kind>(GetCodeKind()),
+ UNINITIALIZED);
+ Handle<Object> probe(
+ known_map_->FindInCodeCache(*factory->compare_ic_symbol(), flags));
+ if (probe->IsCode()) {
+ *code_out = Code::cast(*probe);
+ return true;
+ }
+ return false;
+}
int ICCompareStub::MinorKey() {
@@ -184,6 +219,10 @@
case CompareIC::OBJECTS:
GenerateObjects(masm);
break;
+ case CompareIC::KNOWN_OBJECTS:
+ ASSERT(*known_map_ != NULL);
+ GenerateKnownObjects(masm);
+ break;
default:
UNREACHABLE();
}
=======================================
--- /branches/bleeding_edge/src/code-stubs.h Fri Dec 2 00:06:37 2011
+++ /branches/bleeding_edge/src/code-stubs.h Thu Dec 8 09:17:21 2011
@@ -193,6 +193,17 @@
virtual InlineCacheState GetICState() {
return UNINITIALIZED;
}
+
+ // Add the code to a specialized cache, specific to an individual
+ // stub type. Please note, this method must add the code object to a
+ // roots object, otherwise we will remove the code during GC.
+ virtual void AddToSpecialCache(Handle<Code> new_object) { }
+
+ // Find code in a specialized cache, work is delegated to the specific
stub.
+ virtual bool FindCodeInSpecialCache(Code** code_out) { return false; }
+
+ // If a stub uses a special cache override this.
+ virtual bool UseSpecialCache() { return false; }
// Returns a name for logging/debugging purposes.
SmartArrayPointer<const char> GetName();
@@ -464,6 +475,8 @@
}
virtual void Generate(MacroAssembler* masm);
+
+ void set_known_map(Handle<Map> map) { known_map_ = map; }
private:
class OpField: public BitField<int, 0, 3> { };
@@ -484,12 +497,18 @@
void GenerateStrings(MacroAssembler* masm);
void GenerateObjects(MacroAssembler* masm);
void GenerateMiss(MacroAssembler* masm);
+ void GenerateKnownObjects(MacroAssembler* masm);
bool strict() const { return op_ == Token::EQ_STRICT; }
Condition GetCondition() const { return
CompareIC::ComputeCondition(op_); }
+
+ virtual void AddToSpecialCache(Handle<Code> new_object);
+ virtual bool FindCodeInSpecialCache(Code** code_out);
+ virtual bool UseSpecialCache() { return state_ ==
CompareIC::KNOWN_OBJECTS; }
Token::Value op_;
CompareIC::State state_;
+ Handle<Map> known_map_;
};
=======================================
--- /branches/bleeding_edge/src/heap.cc Wed Dec 7 00:43:18 2011
+++ /branches/bleeding_edge/src/heap.cc Thu Dec 8 09:17:21 2011
@@ -2417,6 +2417,7 @@
}
set_code_stubs(NumberDictionary::cast(obj));
+
// Allocate the non_monomorphic_cache used in stub-cache.cc. The initial
size
// is set to avoid expanding the dictionary during bootstrapping.
{ MaybeObject* maybe_obj = NumberDictionary::Allocate(64);
=======================================
--- /branches/bleeding_edge/src/heap.h Wed Nov 30 03:13:36 2011
+++ /branches/bleeding_edge/src/heap.h Thu Dec 8 09:17:21 2011
@@ -245,6 +245,7 @@
V(use_strict, "use strict") \
V(dot_symbol, ".") \
V(anonymous_function_symbol, "(anonymous function)") \
+ V(compare_ic_symbol, ".compare_ic") \
V(infinity_symbol, "Infinity") \
V(minus_infinity_symbol, "-Infinity")
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Tue Dec 6 07:31:01 2011
+++ /branches/bleeding_edge/src/hydrogen.cc Thu Dec 8 09:17:21 2011
@@ -6141,14 +6141,27 @@
switch (op) {
case Token::EQ:
case Token::EQ_STRICT: {
- AddInstruction(new(zone()) HCheckNonSmi(left));
- AddInstruction(HCheckInstanceType::NewIsSpecObject(left));
- AddInstruction(new(zone()) HCheckNonSmi(right));
- AddInstruction(HCheckInstanceType::NewIsSpecObject(right));
- HCompareObjectEqAndBranch* result =
- new(zone()) HCompareObjectEqAndBranch(left, right);
- result->set_position(expr->position());
- return ast_context()->ReturnControl(result, expr->id());
+ // Can we get away with map check and not instance type check?
+ Handle<Map> map = oracle()->GetCompareMap(expr);
+ if (!map.is_null()) {
+ AddInstruction(new(zone()) HCheckNonSmi(left));
+ AddInstruction(new(zone()) HCheckMap(left, map));
+ AddInstruction(new(zone()) HCheckNonSmi(right));
+ AddInstruction(new(zone()) HCheckMap(right, map));
+ HCompareObjectEqAndBranch* result =
+ new(zone()) HCompareObjectEqAndBranch(left, right);
+ result->set_position(expr->position());
+ return ast_context()->ReturnControl(result, expr->id());
+ } else {
+ AddInstruction(new(zone()) HCheckNonSmi(left));
+ AddInstruction(HCheckInstanceType::NewIsSpecObject(left));
+ AddInstruction(new(zone()) HCheckNonSmi(right));
+ AddInstruction(HCheckInstanceType::NewIsSpecObject(right));
+ HCompareObjectEqAndBranch* result =
+ new(zone()) HCompareObjectEqAndBranch(left, right);
+ result->set_position(expr->position());
+ return ast_context()->ReturnControl(result, expr->id());
+ }
}
default:
return Bailout("Unsupported non-primitive compare");
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Wed Dec 7 08:15:18
2011
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Thu Dec 8 09:17:21
2011
@@ -6670,32 +6670,44 @@
}
-void ICCompareStub::GenerateMiss(MacroAssembler* masm) {
- // Save the registers.
- __ pop(ecx);
- __ push(edx);
- __ push(eax);
- __ push(ecx);
-
+void ICCompareStub::GenerateKnownObjects(MacroAssembler* masm) {
+ Label miss;
+ __ mov(ecx, edx);
+ __ and_(ecx, eax);
+ __ JumpIfSmi(ecx, &miss, Label::kNear);
+
+ __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset));
+ __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset));
+ __ cmp(ecx, known_map_);
+ __ j(not_equal, &miss, Label::kNear);
+ __ cmp(ebx, known_map_);
+ __ j(not_equal, &miss, Label::kNear);
+
+ __ sub(eax, edx);
+ __ ret(0);
+
+ __ bind(&miss);
+ GenerateMiss(masm);
+}
+
+
+void ICCompareStub::GenerateMiss(MacroAssembler* masm) {
{
// Call the runtime system in a fresh internal frame.
ExternalReference miss =
ExternalReference(IC_Utility(IC::kCompareIC_Miss),
masm->isolate());
FrameScope scope(masm, StackFrame::INTERNAL);
- __ push(edx);
+ __ push(edx); // Preserve edx and eax.
+ __ push(eax);
+ __ push(edx); // And also use them as the arguments.
__ push(eax);
__ push(Immediate(Smi::FromInt(op_)));
__ CallExternalReference(miss, 3);
- }
-
- // Compute the entry point of the rewritten stub.
- __ lea(edi, FieldOperand(eax, Code::kHeaderSize));
-
- // Restore registers.
- __ pop(ecx);
- __ pop(eax);
- __ pop(edx);
- __ push(ecx);
+ // Compute the entry point of the rewritten stub.
+ __ lea(edi, FieldOperand(eax, Code::kHeaderSize));
+ __ pop(eax);
+ __ pop(edx);
+ }
// Do a tail call to the rewritten stub.
__ jmp(edi);
=======================================
--- /branches/bleeding_edge/src/ia32/ic-ia32.cc Wed Nov 9 06:32:51 2011
+++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec 8 09:17:21 2011
@@ -1625,6 +1625,9 @@
rewritten = stub.GetCode();
} else {
ICCompareStub stub(op_, state);
+ if (state == KNOWN_OBJECTS) {
+ stub.set_known_map(Handle<Map>(Handle<JSObject>::cast(x)->map()));
+ }
rewritten = stub.GetCode();
}
set_target(*rewritten);
=======================================
--- /branches/bleeding_edge/src/ic.cc Tue Dec 6 04:11:08 2011
+++ /branches/bleeding_edge/src/ic.cc Thu Dec 8 09:17:21 2011
@@ -2320,6 +2320,7 @@
case SMIS: return "SMIS";
case HEAP_NUMBERS: return "HEAP_NUMBERS";
case OBJECTS: return "OBJECTS";
+ case KNOWN_OBJECTS: return "OBJECTS";
case SYMBOLS: return "SYMBOLS";
case STRINGS: return "STRINGS";
case GENERIC: return "GENERIC";
@@ -2334,20 +2335,37 @@
bool has_inlined_smi_code,
Handle<Object> x,
Handle<Object> y) {
- if (!has_inlined_smi_code && state != UNINITIALIZED && state != SYMBOLS)
{
- return GENERIC;
- }
- if (state == UNINITIALIZED && x->IsSmi() && y->IsSmi()) return SMIS;
- if ((state == UNINITIALIZED || (state == SMIS && has_inlined_smi_code))
&&
- x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;
- if (op_ != Token::EQ && op_ != Token::EQ_STRICT) return GENERIC;
- if (state == UNINITIALIZED &&
- x->IsSymbol() && y->IsSymbol()) return SYMBOLS;
- if ((state == UNINITIALIZED || state == SYMBOLS) &&
- x->IsString() && y->IsString()) return STRINGS;
- if (state == UNINITIALIZED &&
- x->IsJSObject() && y->IsJSObject()) return OBJECTS;
- return GENERIC;
+ switch (state) {
+ case UNINITIALIZED:
+ if (x->IsSmi() && y->IsSmi()) return SMIS;
+ if (x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;
+ if (!Token::IsEqualityOp(op_)) return GENERIC;
+ if (x->IsSymbol() && y->IsSymbol()) return SYMBOLS;
+ if (x->IsString() && y->IsString()) return STRINGS;
+ if (x->IsJSObject() && y->IsJSObject()) {
+ if (Handle<JSObject>::cast(x)->map() ==
+ Handle<JSObject>::cast(y)->map() &&
+ Token::IsEqualityOp(op_)) {
+ return KNOWN_OBJECTS;
+ } else {
+ return OBJECTS;
+ }
+ }
+ return GENERIC;
+ case SMIS:
+ return has_inlined_smi_code && x->IsNumber() && y->IsNumber()
+ ? HEAP_NUMBERS
+ : GENERIC;
+ case SYMBOLS:
+ ASSERT(Token::IsEqualityOp(op_));
+ return x->IsString() && y->IsString() ? STRINGS : GENERIC;
+ case HEAP_NUMBERS:
+ case STRINGS:
+ case OBJECTS:
+ case KNOWN_OBJECTS:
+ case GENERIC:
+ return GENERIC;
+ }
}
=======================================
--- /branches/bleeding_edge/src/ic.h Mon Dec 5 13:54:45 2011
+++ /branches/bleeding_edge/src/ic.h Thu Dec 8 09:17:21 2011
@@ -724,6 +724,7 @@
SYMBOLS,
STRINGS,
OBJECTS,
+ KNOWN_OBJECTS,
GENERIC
};
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Thu Dec 8 08:07:07 2011
+++ /branches/bleeding_edge/src/mark-compact.cc Thu Dec 8 09:17:21 2011
@@ -883,8 +883,6 @@
Code* target = Code::GetCodeFromTargetAddress(rinfo->target_address());
if (FLAG_cleanup_code_caches_at_gc && target->is_inline_cache_stub()) {
IC::Clear(rinfo->pc());
- // Please note targets for cleared inline cached do not have to be
- // marked since they are contained in HEAP->non_monomorphic_cache().
target = Code::GetCodeFromTargetAddress(rinfo->target_address());
} else {
if (FLAG_cleanup_code_caches_at_gc &&
@@ -893,9 +891,10 @@
target->has_function_cache()) {
CallFunctionStub::Clear(heap, rinfo->pc());
}
- MarkBit code_mark = Marking::MarkBitFrom(target);
- heap->mark_compact_collector()->MarkObject(target, code_mark);
- }
+ }
+ MarkBit code_mark = Marking::MarkBitFrom(target);
+ heap->mark_compact_collector()->MarkObject(target, code_mark);
+
heap->mark_compact_collector()->RecordRelocSlot(rinfo, target);
}
=======================================
--- /branches/bleeding_edge/src/type-info.cc Fri Nov 18 06:08:57 2011
+++ /branches/bleeding_edge/src/type-info.cc Thu Dec 8 09:17:21 2011
@@ -259,6 +259,7 @@
case CompareIC::STRINGS:
return TypeInfo::String();
case CompareIC::OBJECTS:
+ case CompareIC::KNOWN_OBJECTS:
// TODO(kasperl): We really need a type for JS objects here.
return TypeInfo::NonPrimitive();
case CompareIC::GENERIC:
@@ -276,6 +277,19 @@
CompareIC::State state =
static_cast<CompareIC::State>(code->compare_state());
return state == CompareIC::SYMBOLS;
}
+
+
+Handle<Map> TypeFeedbackOracle::GetCompareMap(CompareOperation* expr) {
+ Handle<Object> object = GetInfo(expr->id());
+ if (!object->IsCode()) return Handle<Map>::null();
+ Handle<Code> code = Handle<Code>::cast(object);
+ if (!code->is_compare_ic_stub()) return Handle<Map>::null();
+ CompareIC::State state =
static_cast<CompareIC::State>(code->compare_state());
+ if (state != CompareIC::KNOWN_OBJECTS) {
+ return Handle<Map>::null();
+ }
+ return Handle<Map>(code->FindFirstMap());
+}
TypeInfo TypeFeedbackOracle::UnaryType(UnaryOperation* expr) {
@@ -367,6 +381,7 @@
case CompareIC::HEAP_NUMBERS:
return TypeInfo::Number();
case CompareIC::OBJECTS:
+ case CompareIC::KNOWN_OBJECTS:
// TODO(kasperl): We really need a type for JS objects here.
return TypeInfo::NonPrimitive();
case CompareIC::GENERIC:
=======================================
--- /branches/bleeding_edge/src/type-info.h Thu Nov 17 05:57:55 2011
+++ /branches/bleeding_edge/src/type-info.h Thu Dec 8 09:17:21 2011
@@ -273,6 +273,7 @@
TypeInfo BinaryType(BinaryOperation* expr);
TypeInfo CompareType(CompareOperation* expr);
bool IsSymbolCompare(CompareOperation* expr);
+ Handle<Map> GetCompareMap(CompareOperation* expr);
TypeInfo SwitchType(CaseClause* clause);
TypeInfo IncrementType(CountOperation* expr);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
