Revert of Speedup access to global_proxy.* attributes/accessors. (patchset #6 id:160001 of https://codereview.chromium.org/2403003002/ )
Reason for revert: Revert, because of crbug.com/656959. Original issue's description: > Speedup access to global_proxy.* attributes/accessors. > > Using a global proxy (e.g. 'window.f', 'w.f' or 'this.f') is considerably slower than evaluating just 'f'. This CL aims to perform the necessary checks at compile time and inline the accesses. > > This is a follow-on CL to crrev.com/2369933005: > - The initial upload is crrev.com/2369933005 + a rebase. > - The remaining issues are the fixes requested by the reviewers on that CL. > > BUG=chromium:634276, chromium:654716 > > Committed: https://crrev.com/8f43d748272536117008aa6a1b53ea52126261c1 > Committed: https://crrev.com/041314524952a3c1bc71bd3beafbbb37319f1d22 > Cr-Original-Commit-Position: refs/heads/master@{#40153} > Cr-Commit-Position: refs/heads/master@{#40365} TBR=jochen@chromium.org,verwaest@chromium.org NOTRY=true NOPRESUBMIT=true BUG=chromium:634276, chromium:654716 Review-Url: https://chromiumcodereview.appspot.com/2434233002 Cr-Commit-Position: refs/heads/master@{#40481}
This commit is contained in:
vogelheim
Commit bot
parent
fa0f953297
commit
9e6bfbd33c
@ -5264,16 +5264,12 @@ void HOptimizedGraphBuilder::VisitConditional(Conditional* expr) {
|
||||
}
|
||||
}
|
||||
|
||||
bool HOptimizedGraphBuilder::CanInlineGlobalPropertyAccess(
|
||||
Variable* var, LookupIterator* it, PropertyAccessType access_type) {
|
||||
if (var->is_this()) return false;
|
||||
return CanInlineGlobalPropertyAccess(it, access_type);
|
||||
}
|
||||
|
||||
bool HOptimizedGraphBuilder::CanInlineGlobalPropertyAccess(
|
||||
LookupIterator* it, PropertyAccessType access_type) {
|
||||
if (!current_info()->has_global_object()) {
|
||||
return false;
|
||||
HOptimizedGraphBuilder::GlobalPropertyAccess
|
||||
HOptimizedGraphBuilder::LookupGlobalProperty(Variable* var, LookupIterator* it,
|
||||
PropertyAccessType access_type) {
|
||||
if (var->is_this() || !current_info()->has_global_object()) {
|
||||
return kUseGeneric;
|
||||
}
|
||||
|
||||
switch (it->state()) {
|
||||
@ -5282,17 +5278,17 @@ bool HOptimizedGraphBuilder::CanInlineGlobalPropertyAccess(
|
||||
case LookupIterator::INTERCEPTOR:
|
||||
case LookupIterator::INTEGER_INDEXED_EXOTIC:
|
||||
case LookupIterator::NOT_FOUND:
|
||||
return false;
|
||||
return kUseGeneric;
|
||||
case LookupIterator::DATA:
|
||||
if (access_type == STORE && it->IsReadOnly()) return false;
|
||||
if (!it->GetHolder<JSObject>()->IsJSGlobalObject()) return false;
|
||||
return true;
|
||||
if (access_type == STORE && it->IsReadOnly()) return kUseGeneric;
|
||||
if (!it->GetHolder<JSObject>()->IsJSGlobalObject()) return kUseGeneric;
|
||||
return kUseCell;
|
||||
case LookupIterator::JSPROXY:
|
||||
case LookupIterator::TRANSITION:
|
||||
UNREACHABLE();
|
||||
}
|
||||
UNREACHABLE();
|
||||
return false;
|
||||
return kUseGeneric;
|
||||
}
|
||||
|
||||
|
||||
@ -5308,55 +5304,6 @@ HValue* HOptimizedGraphBuilder::BuildContextChainWalk(Variable* var) {
|
||||
return context;
|
||||
}
|
||||
|
||||
void HOptimizedGraphBuilder::InlineGlobalPropertyLoad(LookupIterator* it,
|
||||
BailoutId ast_id) {
|
||||
Handle<PropertyCell> cell = it->GetPropertyCell();
|
||||
top_info()->dependencies()->AssumePropertyCell(cell);
|
||||
auto cell_type = it->property_details().cell_type();
|
||||
if (cell_type == PropertyCellType::kConstant ||
|
||||
cell_type == PropertyCellType::kUndefined) {
|
||||
Handle<Object> constant_object(cell->value(), isolate());
|
||||
if (constant_object->IsConsString()) {
|
||||
constant_object = String::Flatten(Handle<String>::cast(constant_object));
|
||||
}
|
||||
HConstant* constant = New<HConstant>(constant_object);
|
||||
return ast_context()->ReturnInstruction(constant, ast_id);
|
||||
} else {
|
||||
auto access = HObjectAccess::ForPropertyCellValue();
|
||||
UniqueSet<Map>* field_maps = nullptr;
|
||||
if (cell_type == PropertyCellType::kConstantType) {
|
||||
switch (cell->GetConstantType()) {
|
||||
case PropertyCellConstantType::kSmi:
|
||||
access = access.WithRepresentation(Representation::Smi());
|
||||
break;
|
||||
case PropertyCellConstantType::kStableMap: {
|
||||
// Check that the map really is stable. The heap object could
|
||||
// have mutated without the cell updating state. In that case,
|
||||
// make no promises about the loaded value except that it's a
|
||||
// heap object.
|
||||
access = access.WithRepresentation(Representation::HeapObject());
|
||||
Handle<Map> map(HeapObject::cast(cell->value())->map());
|
||||
if (map->is_stable()) {
|
||||
field_maps = new (zone())
|
||||
UniqueSet<Map>(Unique<Map>::CreateImmovable(map), zone());
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
HConstant* cell_constant = Add<HConstant>(cell);
|
||||
HLoadNamedField* instr;
|
||||
if (field_maps == nullptr) {
|
||||
instr = New<HLoadNamedField>(cell_constant, nullptr, access);
|
||||
} else {
|
||||
instr = New<HLoadNamedField>(cell_constant, nullptr, access, field_maps,
|
||||
HType::HeapObject());
|
||||
}
|
||||
instr->ClearDependsOnFlag(kInobjectFields);
|
||||
instr->SetDependsOnFlag(kGlobalVars);
|
||||
return ast_context()->ReturnInstruction(instr, ast_id);
|
||||
}
|
||||
}
|
||||
|
||||
void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
|
||||
DCHECK(!HasStackOverflow());
|
||||
@ -5405,9 +5352,57 @@ void HOptimizedGraphBuilder::VisitVariableProxy(VariableProxy* expr) {
|
||||
}
|
||||
|
||||
LookupIterator it(global, variable->name(), LookupIterator::OWN);
|
||||
if (CanInlineGlobalPropertyAccess(variable, &it, LOAD)) {
|
||||
InlineGlobalPropertyLoad(&it, expr->id());
|
||||
return;
|
||||
GlobalPropertyAccess type = LookupGlobalProperty(variable, &it, LOAD);
|
||||
|
||||
if (type == kUseCell) {
|
||||
Handle<PropertyCell> cell = it.GetPropertyCell();
|
||||
top_info()->dependencies()->AssumePropertyCell(cell);
|
||||
auto cell_type = it.property_details().cell_type();
|
||||
if (cell_type == PropertyCellType::kConstant ||
|
||||
cell_type == PropertyCellType::kUndefined) {
|
||||
Handle<Object> constant_object(cell->value(), isolate());
|
||||
if (constant_object->IsConsString()) {
|
||||
constant_object =
|
||||
String::Flatten(Handle<String>::cast(constant_object));
|
||||
}
|
||||
HConstant* constant = New<HConstant>(constant_object);
|
||||
return ast_context()->ReturnInstruction(constant, expr->id());
|
||||
} else {
|
||||
auto access = HObjectAccess::ForPropertyCellValue();
|
||||
UniqueSet<Map>* field_maps = nullptr;
|
||||
if (cell_type == PropertyCellType::kConstantType) {
|
||||
switch (cell->GetConstantType()) {
|
||||
case PropertyCellConstantType::kSmi:
|
||||
access = access.WithRepresentation(Representation::Smi());
|
||||
break;
|
||||
case PropertyCellConstantType::kStableMap: {
|
||||
// Check that the map really is stable. The heap object could
|
||||
// have mutated without the cell updating state. In that case,
|
||||
// make no promises about the loaded value except that it's a
|
||||
// heap object.
|
||||
access =
|
||||
access.WithRepresentation(Representation::HeapObject());
|
||||
Handle<Map> map(HeapObject::cast(cell->value())->map());
|
||||
if (map->is_stable()) {
|
||||
field_maps = new (zone())
|
||||
UniqueSet<Map>(Unique<Map>::CreateImmovable(map), zone());
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
HConstant* cell_constant = Add<HConstant>(cell);
|
||||
HLoadNamedField* instr;
|
||||
if (field_maps == nullptr) {
|
||||
instr = New<HLoadNamedField>(cell_constant, nullptr, access);
|
||||
} else {
|
||||
instr = New<HLoadNamedField>(cell_constant, nullptr, access,
|
||||
field_maps, HType::HeapObject());
|
||||
}
|
||||
instr->ClearDependsOnFlag(kInobjectFields);
|
||||
instr->SetDependsOnFlag(kGlobalVars);
|
||||
return ast_context()->ReturnInstruction(instr, expr->id());
|
||||
}
|
||||
} else {
|
||||
Handle<TypeFeedbackVector> vector(current_feedback_vector(), isolate());
|
||||
|
||||
@ -6511,58 +6506,6 @@ void HOptimizedGraphBuilder::HandlePropertyAssignment(Assignment* expr) {
|
||||
expr->AssignmentId(), expr->IsUninitialized());
|
||||
}
|
||||
|
||||
HInstruction* HOptimizedGraphBuilder::InlineGlobalPropertyStore(
|
||||
LookupIterator* it, HValue* value, BailoutId ast_id) {
|
||||
Handle<PropertyCell> cell = it->GetPropertyCell();
|
||||
top_info()->dependencies()->AssumePropertyCell(cell);
|
||||
auto cell_type = it->property_details().cell_type();
|
||||
if (cell_type == PropertyCellType::kConstant ||
|
||||
cell_type == PropertyCellType::kUndefined) {
|
||||
Handle<Object> constant(cell->value(), isolate());
|
||||
if (value->IsConstant()) {
|
||||
HConstant* c_value = HConstant::cast(value);
|
||||
if (!constant.is_identical_to(c_value->handle(isolate()))) {
|
||||
Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
|
||||
Deoptimizer::EAGER);
|
||||
}
|
||||
} else {
|
||||
HValue* c_constant = Add<HConstant>(constant);
|
||||
IfBuilder builder(this);
|
||||
if (constant->IsNumber()) {
|
||||
builder.If<HCompareNumericAndBranch>(value, c_constant, Token::EQ);
|
||||
} else {
|
||||
builder.If<HCompareObjectEqAndBranch>(value, c_constant);
|
||||
}
|
||||
builder.Then();
|
||||
builder.Else();
|
||||
Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
|
||||
Deoptimizer::EAGER);
|
||||
builder.End();
|
||||
}
|
||||
}
|
||||
HConstant* cell_constant = Add<HConstant>(cell);
|
||||
auto access = HObjectAccess::ForPropertyCellValue();
|
||||
if (cell_type == PropertyCellType::kConstantType) {
|
||||
switch (cell->GetConstantType()) {
|
||||
case PropertyCellConstantType::kSmi:
|
||||
access = access.WithRepresentation(Representation::Smi());
|
||||
break;
|
||||
case PropertyCellConstantType::kStableMap: {
|
||||
// The map may no longer be stable, deopt if it's ever different from
|
||||
// what is currently there, which will allow for restablization.
|
||||
Handle<Map> map(HeapObject::cast(cell->value())->map());
|
||||
Add<HCheckHeapObject>(value);
|
||||
value = Add<HCheckMaps>(value, map);
|
||||
access = access.WithRepresentation(Representation::HeapObject());
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
HInstruction* instr = New<HStoreNamedField>(cell_constant, access, value);
|
||||
instr->ClearChangesFlag(kInobjectFields);
|
||||
instr->SetChangesFlag(kGlobalVars);
|
||||
return instr;
|
||||
}
|
||||
|
||||
// Because not every expression has a position and there is not common
|
||||
// superclass of Assignment and CountOperation, we cannot just pass the
|
||||
@ -6603,9 +6546,56 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
|
||||
}
|
||||
|
||||
LookupIterator it(global, var->name(), LookupIterator::OWN);
|
||||
if (CanInlineGlobalPropertyAccess(var, &it, STORE)) {
|
||||
HInstruction* instr = InlineGlobalPropertyStore(&it, value, ast_id);
|
||||
AddInstruction(instr);
|
||||
GlobalPropertyAccess type = LookupGlobalProperty(var, &it, STORE);
|
||||
if (type == kUseCell) {
|
||||
Handle<PropertyCell> cell = it.GetPropertyCell();
|
||||
top_info()->dependencies()->AssumePropertyCell(cell);
|
||||
auto cell_type = it.property_details().cell_type();
|
||||
if (cell_type == PropertyCellType::kConstant ||
|
||||
cell_type == PropertyCellType::kUndefined) {
|
||||
Handle<Object> constant(cell->value(), isolate());
|
||||
if (value->IsConstant()) {
|
||||
HConstant* c_value = HConstant::cast(value);
|
||||
if (!constant.is_identical_to(c_value->handle(isolate()))) {
|
||||
Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
|
||||
Deoptimizer::EAGER);
|
||||
}
|
||||
} else {
|
||||
HValue* c_constant = Add<HConstant>(constant);
|
||||
IfBuilder builder(this);
|
||||
if (constant->IsNumber()) {
|
||||
builder.If<HCompareNumericAndBranch>(value, c_constant, Token::EQ);
|
||||
} else {
|
||||
builder.If<HCompareObjectEqAndBranch>(value, c_constant);
|
||||
}
|
||||
builder.Then();
|
||||
builder.Else();
|
||||
Add<HDeoptimize>(DeoptimizeReason::kConstantGlobalVariableAssignment,
|
||||
Deoptimizer::EAGER);
|
||||
builder.End();
|
||||
}
|
||||
}
|
||||
HConstant* cell_constant = Add<HConstant>(cell);
|
||||
auto access = HObjectAccess::ForPropertyCellValue();
|
||||
if (cell_type == PropertyCellType::kConstantType) {
|
||||
switch (cell->GetConstantType()) {
|
||||
case PropertyCellConstantType::kSmi:
|
||||
access = access.WithRepresentation(Representation::Smi());
|
||||
break;
|
||||
case PropertyCellConstantType::kStableMap: {
|
||||
// The map may no longer be stable, deopt if it's ever different from
|
||||
// what is currently there, which will allow for restablization.
|
||||
Handle<Map> map(HeapObject::cast(cell->value())->map());
|
||||
Add<HCheckHeapObject>(value);
|
||||
value = Add<HCheckMaps>(value, map);
|
||||
access = access.WithRepresentation(Representation::HeapObject());
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
HInstruction* instr = Add<HStoreNamedField>(cell_constant, access, value);
|
||||
instr->ClearChangesFlag(kInobjectFields);
|
||||
instr->SetChangesFlag(kGlobalVars);
|
||||
if (instr->HasObservableSideEffects()) {
|
||||
Add<HSimulate>(ast_id, REMOVABLE_SIMULATE);
|
||||
}
|
||||
@ -7528,37 +7518,6 @@ HValue* HOptimizedGraphBuilder::BuildNamedAccess(
|
||||
DCHECK(maps != NULL);
|
||||
|
||||
if (maps->length() > 0) {
|
||||
Handle<JSGlobalObject> global_object(current_info()->global_object());
|
||||
Handle<Context> current_context(current_info()->context());
|
||||
Handle<JSObject> global_proxy(current_context->global_proxy());
|
||||
|
||||
// Check for special case: Access via a single map to the global proxy
|
||||
// can also be handled monomorphically.
|
||||
Handle<Object> map_constructor =
|
||||
handle(maps->first()->GetConstructor(), isolate());
|
||||
if (map_constructor->IsJSFunction()) {
|
||||
Handle<Context> map_context =
|
||||
handle(Handle<JSFunction>::cast(map_constructor)->context());
|
||||
bool is_global_proxy_access =
|
||||
maps->length() == 1 && // More than one map, fallback to polymorphic?
|
||||
maps->first()->IsJSGlobalProxyMap() &&
|
||||
isolate()->MayAccess(map_context, global_proxy);
|
||||
|
||||
if (is_global_proxy_access) {
|
||||
LookupIterator it(global_object, name, LookupIterator::OWN);
|
||||
if (CanInlineGlobalPropertyAccess(&it, access)) {
|
||||
BuildCheckHeapObject(object);
|
||||
Add<HCheckMaps>(object, maps);
|
||||
if (access == LOAD) {
|
||||
InlineGlobalPropertyLoad(&it, expr->id());
|
||||
return nullptr;
|
||||
} else {
|
||||
return InlineGlobalPropertyStore(&it, value, expr->id());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
PropertyAccessInfo info(this, access, maps->first(), name);
|
||||
if (!info.CanAccessAsMonomorphic(maps)) {
|
||||
HandlePolymorphicNamedFieldAccess(access, expr, slot, ast_id, return_id,
|
||||
|
||||
@ -2340,15 +2340,13 @@ class HOptimizedGraphBuilder : public HGraphBuilder,
|
||||
#undef DECLARE_VISIT
|
||||
|
||||
private:
|
||||
bool CanInlineGlobalPropertyAccess(Variable* var, LookupIterator* it,
|
||||
PropertyAccessType access_type);
|
||||
|
||||
bool CanInlineGlobalPropertyAccess(LookupIterator* it,
|
||||
PropertyAccessType access_type);
|
||||
|
||||
void InlineGlobalPropertyLoad(LookupIterator* it, BailoutId ast_id);
|
||||
HInstruction* InlineGlobalPropertyStore(LookupIterator* it, HValue* value,
|
||||
BailoutId ast_id);
|
||||
// Helpers for flow graph construction.
|
||||
enum GlobalPropertyAccess {
|
||||
kUseCell,
|
||||
kUseGeneric
|
||||
};
|
||||
GlobalPropertyAccess LookupGlobalProperty(Variable* var, LookupIterator* it,
|
||||
PropertyAccessType access_type);
|
||||
|
||||
void EnsureArgumentsArePushedForAccess();
|
||||
bool TryArgumentsAccess(Property* expr);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user