Skip to content

V
V8
Commit 12e2f1ac authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot
Browse files
Options

[ic] Various cleanups to the IC class. 

This includes a bunch of individual changes:

 - Refactor IC::OnFeedbackChanged() and remove the now unused
   GetHostFunction().
 - Remove constant_pool, pc and fp from the IC, since it was
   only used for --trace-ic by now, and here we can now use the
   JavaScriptFrameIterator instead.
 - Use FLAG_max_polymorphic_map_count consistently instead of having a
   separate kMaxKeyedPolymorphism constant in some cases.
 - Remove dead IC::TraceHandlerCacheHitStats() method.
 - Remove the unused maybe_handler_ field.

Bug: v8:9183
Change-Id: I29680519ab84b2790c73f50fca966a66918a7e31
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1649567
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62083}
parent d0d87706
Show whitespace changes
Inline Side-by-side
Showing with 33 additions and 152 deletions
src/ic/ic-inl.h
View file @ 12e2f1ac
......@@ -16,23 +16,6 @@
namespace v8 {
namespace internal {
Address IC::constant_pool() const {
if (FLAG_enable_embedded_constant_pool) {
return raw_constant_pool();
} else {
return kNullAddress;
}
}
Address IC::raw_constant_pool() const {
if (FLAG_enable_embedded_constant_pool) {
return *constant_pool_address_;
} else {
return kNullAddress;
}
}
void IC::update_receiver_map(Handle<Object> receiver) {
if (receiver->IsSmi()) {
receiver_map_ = isolate_->factory()->heap_number_map();
......@@ -50,13 +33,6 @@ bool IC::IsHandler(MaybeObject object) {
(heap_object.IsDataHandler() || heap_object.IsCode()));
}
bool IC::HostIsDeoptimizedCode() const {
Code host =
isolate()->inner_pointer_to_code_cache()->GetCacheEntry(pc())->code;
return (host.kind() == Code::OPTIMIZED_FUNCTION &&
host.marked_for_deoptimization());
}
bool IC::vector_needs_update() {
if (state() == NO_FEEDBACK) return false;
return (!vector_set_ &&
......
src/ic/ic.cc
View file @ 12e2f1ac
......@@ -29,9 +29,7 @@
#include "src/objects/js-array-inl.h"
#include "src/objects/module-inl.h"
#include "src/objects/struct-inl.h"
#ifdef V8_TRACE_FEEDBACK_UPDATES
#include "src/utils/ostreams.h"
#endif // V8_TRACE_FEEDBACK_UPDATES
#include "src/execution/runtime-profiler.h"
#include "src/objects/prototype.h"
#include "src/runtime/runtime-utils.h"
......@@ -89,7 +87,6 @@ const char* GetModifier(KeyedAccessStoreMode mode) {
void IC::TraceIC(const char* type, Handle<Object> name) {
if (V8_LIKELY(!TracingFlags::is_ic_stats_enabled())) return;
if (HostIsDeoptimizedCode()) return;
State new_state =
(state() == NO_FEEDBACK) ? NO_FEEDBACK : nexus()->ic_state();
TraceIC(type, name, state(), new_state);
......@@ -126,20 +123,21 @@ void IC::TraceIC(const char* type, Handle<Object> name, State old_state,
return;
}
JavaScriptFrameIterator it(isolate());
JavaScriptFrame* frame = it.frame();
JSFunction function = frame->function();
ICStats::instance()->Begin();
ICInfo& ic_info = ICStats::instance()->Current();
ic_info.type = keyed_prefix ? "Keyed" : "";
ic_info.type += type;
Object maybe_function =
Object(Memory<Address>(fp_ + JavaScriptFrameConstants::kFunctionOffset));
DCHECK(maybe_function.IsJSFunction());
JSFunction function = JSFunction::cast(maybe_function);
int code_offset = 0;
if (function.IsInterpreted()) {
code_offset = InterpretedFrame::GetBytecodeOffset(fp());
code_offset = InterpretedFrame::GetBytecodeOffset(frame->fp());
} else {
code_offset = static_cast<int>(pc() - function.code().InstructionStart());
code_offset =
static_cast<int>(frame->pc() - function.code().InstructionStart());
}
JavaScriptFrame::CollectFunctionAndOffsetForICStats(
function, function.abstract_code(), code_offset);
......@@ -170,56 +168,11 @@ IC::IC(Isolate* isolate, Handle<FeedbackVector> vector, FeedbackSlot slot,
target_maps_set_(false),
slow_stub_reason_(nullptr),
nexus_(vector, slot) {
// To improve the performance of the (much used) IC code, we unfold a few
// levels of the stack frame iteration code. This yields a ~35% speedup when
// running DeltaBlue and a ~25% speedup of gbemu with the '--nouse-ic' flag.
const Address entry = Isolate::c_entry_fp(isolate->thread_local_top());
Address* constant_pool = nullptr;
if (FLAG_enable_embedded_constant_pool) {
constant_pool = reinterpret_cast<Address*>(
entry + ExitFrameConstants::kConstantPoolOffset);
}
Address* pc_address =
reinterpret_cast<Address*>(entry + ExitFrameConstants::kCallerPCOffset);
Address fp = Memory<Address>(entry + ExitFrameConstants::kCallerFPOffset);
#ifdef DEBUG
StackFrameIterator it(isolate);
for (int i = 0; i < 1; i++) it.Advance();
StackFrame* frame = it.frame();
DCHECK(fp == frame->fp() && pc_address == frame->pc_address());
#endif
// For interpreted functions, some bytecode handlers construct a
// frame. We have to skip the constructed frame to find the interpreted
// function's frame. Check if the there is an additional frame, and if there
// is skip this frame. However, the pc should not be updated. The call to
// ICs happen from bytecode handlers.
intptr_t frame_marker =
Memory<intptr_t>(fp + TypedFrameConstants::kFrameTypeOffset);
if (frame_marker == StackFrame::TypeToMarker(StackFrame::STUB)) {
fp = Memory<Address>(fp + TypedFrameConstants::kCallerFPOffset);
}
fp_ = fp;
if (FLAG_enable_embedded_constant_pool) {
constant_pool_address_ = constant_pool;
}
pc_address_ = StackFrame::ResolveReturnAddressLocation(pc_address);
DCHECK_IMPLIES(!vector.is_null(), kind_ == nexus_.kind());
state_ = (vector.is_null()) ? NO_FEEDBACK : nexus_.ic_state();
old_state_ = state_;
}
JSFunction IC::GetHostFunction() const {
// Compute the JavaScript frame for the frame pointer of this IC
// structure. We need this to be able to find the function
// corresponding to the frame.
StackFrameIterator it(isolate());
while (it.frame()->fp() != this->fp()) it.Advance();
JavaScriptFrame* frame = JavaScriptFrame::cast(it.frame());
// Find the function on the stack and both the active code for the
// function and the original code.
return frame->function();
}
static void LookupForRead(LookupIterator* it, bool is_has_property) {
for (; it->IsFound(); it->Next()) {
switch (it->state()) {
......@@ -262,14 +215,14 @@ bool IC::ShouldRecomputeHandler(Handle<String> name) {
// monomorphic.
if (IsGlobalIC()) return true;
maybe_handler_ = nexus()->FindHandlerForMap(receiver_map());
MaybeObjectHandle maybe_handler = nexus()->FindHandlerForMap(receiver_map());
// The current map wasn't handled yet. There's no reason to stay monomorphic,
// *unless* we're moving from a deprecated map to its replacement, or
// to a more general elements kind.
// TODO(verwaest): Check if the current map is actually what the old map
// would transition to.
if (maybe_handler_.is_null()) {
if (maybe_handler.is_null()) {
if (!receiver_map()->IsJSObjectMap()) return false;
Map first_map = FirstTargetMap();
if (first_map.is_null()) return false;
......@@ -320,27 +273,23 @@ MaybeHandle<Object> IC::ReferenceError(Handle<Name> name) {
isolate(), NewReferenceError(MessageTemplate::kNotDefined, name), Object);
}
// static
void IC::OnFeedbackChanged(Isolate* isolate, FeedbackNexus* nexus,
JSFunction host_function, const char* reason) {
FeedbackVector vector = nexus->vector();
FeedbackSlot slot = nexus->slot();
OnFeedbackChanged(isolate, vector, slot, host_function, reason);
void IC::OnFeedbackChanged(const char* reason) {
vector_set_ = true;
FeedbackVector vector = nexus()->vector();
FeedbackSlot slot = nexus()->slot();
OnFeedbackChanged(isolate(), vector, slot, reason);
}
// static
void IC::OnFeedbackChanged(Isolate* isolate, FeedbackVector vector,
FeedbackSlot slot, JSFunction host_function,
const char* reason) {
FeedbackSlot slot, const char* reason) {
if (FLAG_trace_opt_verbose) {
// TODO(leszeks): The host function is only needed for this print, we could
// remove it as a parameter if we're of with removing this trace (or only
// tracing the feedback vector, not the function name).
if (vector.profiler_ticks() != 0) {
PrintF("[resetting ticks for ");
host_function.ShortPrint();
PrintF(" due from %d due to IC change: %s]\n", vector.profiler_ticks(),
reason);
StdoutStream os;
os << "[resetting ticks for ";
vector.shared_function_info().ShortPrint(os);
os << " from " << vector.profiler_ticks()
<< " due to IC change: " << reason << "]" << std::endl;
}
}
vector.set_profiler_ticks(0);
......@@ -348,7 +297,6 @@ void IC::OnFeedbackChanged(Isolate* isolate, FeedbackVector vector,
#ifdef V8_TRACE_FEEDBACK_UPDATES
if (FLAG_trace_feedback_updates) {
int slot_count = vector.metadata().slot_count();
StdoutStream os;
if (slot.IsInvalid()) {
os << "[Feedback slots in ";
......@@ -386,15 +334,13 @@ bool IC::ConfigureVectorState(IC::State new_state, Handle<Object> key) {
// functions doesn't improve performance.
bool changed =
nexus()->ConfigureMegamorphic(key->IsName() ? PROPERTY : ELEMENT);
vector_set_ = true;
OnFeedbackChanged(isolate(), nexus(), GetHostFunction(), "Megamorphic");
OnFeedbackChanged("Megamorphic");
return changed;
}
void IC::ConfigureVectorState(Handle<Map> map) {
nexus()->ConfigurePremonomorphic(map);
vector_set_ = true;
OnFeedbackChanged(isolate(), nexus(), GetHostFunction(), "Premonomorphic");
OnFeedbackChanged("Premonomorphic");
}
void IC::ConfigureVectorState(Handle<Name> name, Handle<Map> map,
......@@ -412,9 +358,7 @@ void IC::ConfigureVectorState(Handle<Name> name, Handle<Map> map,
nexus()->ConfigureMonomorphic(name, map, handler);
}
vector_set_ = true;
OnFeedbackChanged(isolate(), nexus(), GetHostFunction(),
IsLoadGlobalIC() ? "LoadGlobal" : "Monomorphic");
OnFeedbackChanged(IsLoadGlobalIC() ? "LoadGlobal" : "Monomorphic");
}
void IC::ConfigureVectorState(Handle<Name> name, MapHandles const& maps,
......@@ -424,8 +368,7 @@ void IC::ConfigureVectorState(Handle<Name> name, MapHandles const& maps,
if (!is_keyed()) name = Handle<Name>::null();
nexus()->ConfigurePolymorphic(name, maps, handlers);
vector_set_ = true;
OnFeedbackChanged(isolate(), nexus(), GetHostFunction(), "Polymorphic");
OnFeedbackChanged("Polymorphic");
}
MaybeHandle<Object> LoadIC::Load(Handle<Object> object, Handle<Name> name) {
......@@ -758,17 +701,6 @@ void IC::UpdateMegamorphicCache(Handle<Map> map, Handle<Name> name,
}
}
void IC::TraceHandlerCacheHitStats(LookupIterator* lookup) {
DCHECK_EQ(LookupIterator::ACCESSOR, lookup->state());
if (V8_LIKELY(!TracingFlags::is_runtime_stats_enabled())) return;
if (IsAnyLoad() || IsAnyHas()) {
TRACE_HANDLER_STATS(isolate(), LoadIC_HandlerCacheHit_Accessor);
} else {
DCHECK(IsAnyStore());
TRACE_HANDLER_STATS(isolate(), StoreIC_HandlerCacheHit_Accessor);
}
}
Handle<Object> LoadIC::ComputeHandler(LookupIterator* lookup) {
Handle<Object> receiver = lookup->GetReceiver();
ReadOnlyRoots roots(isolate());
......@@ -1086,7 +1018,8 @@ void KeyedLoadIC::UpdateLoadElement(Handle<HeapObject> receiver,
// If the maximum number of receiver maps has been exceeded, use the generic
// version of the IC.
if (target_receiver_maps.size() > kMaxKeyedPolymorphism) {
if (static_cast<int>(target_receiver_maps.size()) >
FLAG_max_polymorphic_map_count) {
set_slow_stub_reason("max polymorph exceeded");
return;
}
......@@ -1831,7 +1764,10 @@ void KeyedStoreIC::UpdateStoreElement(Handle<Map> receiver_map,
// If the maximum number of receiver maps has been exceeded, use the
// megamorphic version of the IC.
if (target_receiver_maps.size() > kMaxKeyedPolymorphism) return;
if (static_cast<int>(target_receiver_maps.size()) >
FLAG_max_polymorphic_map_count) {
return;
}
// Make sure all polymorphic handlers have the same store mode, otherwise the
// megamorphic stub must be used.
......
src/ic/ic.h
View file @ 12e2f1ac
......@@ -29,8 +29,6 @@ class IC {
// Alias the inline cache state type to make the IC code more readable.
using State = InlineCacheState;
static constexpr int kMaxKeyedPolymorphism = 4;
// Construct the IC structure with the given number of extra
// JavaScript frames on the stack.
IC(Isolate* isolate, Handle<FeedbackVector> vector, FeedbackSlot slot,
......@@ -62,25 +60,15 @@ class IC {
// Nofity the IC system that a feedback has changed.
static void OnFeedbackChanged(Isolate* isolate, FeedbackVector vector,
FeedbackSlot slot, JSFunction host_function,
const char* reason);
FeedbackSlot slot, const char* reason);
static void OnFeedbackChanged(Isolate* isolate, FeedbackNexus* nexus,
JSFunction host_function, const char* reason);
void OnFeedbackChanged(const char* reason);
protected:
Address fp() const { return fp_; }
Address pc() const { return *pc_address_; }
void set_slow_stub_reason(const char* reason) { slow_stub_reason_ = reason; }
Isolate* isolate() const { return isolate_; }
// Get the caller function object.
JSFunction GetHostFunction() const;
inline bool HostIsDeoptimizedCode() const;
bool is_vector_set() { return vector_set_; }
inline bool vector_needs_update();
......@@ -106,8 +94,6 @@ class IC {
Handle<Object> key);
MaybeHandle<Object> ReferenceError(Handle<Name> name);
void TraceHandlerCacheHitStats(LookupIterator* lookup);
void UpdateMonomorphicIC(const MaybeObjectHandle& handler, Handle<Name> name);
bool UpdatePolymorphicIC(Handle<Name> name, const MaybeObjectHandle& handler);
void UpdateMegamorphicCache(Handle<Map> map, Handle<Name> name,
......@@ -158,28 +144,12 @@ class IC {
FeedbackNexus* nexus() { return &nexus_; }
private:
inline Address constant_pool() const;
inline Address raw_constant_pool() const;
void FindTargetMaps() {
if (target_maps_set_) return;
target_maps_set_ = true;
nexus()->ExtractMaps(&target_maps_);
}
// Frame pointer for the frame that uses (calls) the IC.
Address fp_;
// All access to the program counter and constant pool of an IC structure is
// indirect to make the code GC safe. This feature is crucial since
// GetProperty and SetProperty are called and they in turn might
// invoke the garbage collector.
Address* pc_address_;
// The constant pool of the code which originally called the IC (which might
// be for the breakpointed copy of the original code).
Address* constant_pool_address_;
Isolate* isolate_;
bool vector_set_;
......@@ -187,7 +157,6 @@ class IC {
State state_;
FeedbackSlotKind kind_;
Handle<Map> receiver_map_;
MaybeObjectHandle maybe_handler_;
MapHandles target_maps_;
bool target_maps_set_;
......
src/objects/js-objects.cc
View file @ 12e2f1ac
......@@ -5530,7 +5530,7 @@ void JSFunction::ClearTypeFeedbackInfo() {
FeedbackVector vector = feedback_vector();
Isolate* isolate = GetIsolate();
if (vector.ClearSlots(isolate)) {
IC::OnFeedbackChanged(isolate, vector, FeedbackSlot::Invalid(), *this,
IC::OnFeedbackChanged(isolate, vector, FeedbackSlot::Invalid(),
"ClearTypeFeedbackInfo");
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment