Deoptimization fix for Shark
Gary Benson
gbenson at redhat.com
Mon Apr 27 04:22:39 PDT 2009
Hi all,
Deoptimization in HotSpot is where you assume you can optimize code
in a certain way, inserting a runtime check called a trap which will
force the method to be recompiled if the assumption failed. Shark's
traps were firing early, meaning that sometimes they were firing when
they didn't need to and causing methods to be recompiled endlessly.
This commit fixes.
Cheers,
Gary
--
http://gbenson.net/
-------------- next part --------------
diff -r db61663b8232 -r 010cb02d0958 ChangeLog
--- a/ChangeLog Mon Apr 27 05:35:03 2009 -0400
+++ b/ChangeLog Mon Apr 27 07:13:07 2009 -0400
@@ -1,3 +1,34 @@
+2009-04-27 Gary Benson <gbenson at redhat.com>
+
+ * ports/hotspot/src/share/vm/shark/sharkBlock.hpp
+ (SharkBlock::has_trap): New method.
+ (SharkBlock::trap_request): Likewise.
+ (SharkBlock::trap_bci): Likewise.
+ (SharkBlock::do_trap): Likewise.
+ * ports/hotspot/src/share/vm/shark/sharkBlock.cpp
+ (SharkBlock::has_trap): New method.
+ (SharkBlock::trap_request): Likewise.
+ (SharkBlock::trap_bci): Likewise.
+ (SharkBlock::do_trap): Likewise.
+ (SharkBlock::parse_bytecode): Handle traps.
+ * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
+ (SharkTopLevelBlock::_has_trap): New field.
+ (SharkTopLevelBlock::_trap_bci): Likewise.
+ (SharkTopLevelBlock::has-trap): Updated.
+ (SharkTopLevelBlock::trap_request): Likewise.
+ (SharkTopLevelBlock::set_trap): New method.
+ (SharkTopLevelBlock::trap_bci): Likewise.
+ (SharkTopLevelBlock::do_trap): Likewise.
+ (SharkTopLevelBlock::scan_for_traps): New prototype.
+ * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
+ (SharkTopLevelBlock::scan_for_traps): Store trap_bci along
+ with trap_request, and don't assume that traps found by
+ typeflow are the first traps in the block.
+ (SharkTopLevelBlock::enter): Scan for traps, and always
+ enter exception sucessors.
+ (SharkTopLevelBlock::emit_IR): Remove old trap handling.
+ (SharkTopLevelBlock::do_trap): New method.
+
2009-04-27 Gary Benson <gbenson at redhat.com>
* ports/hotspot/src/share/vm/shark/sharkState.hpp
diff -r db61663b8232 -r 010cb02d0958 ports/hotspot/src/share/vm/shark/sharkBlock.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkBlock.cpp Mon Apr 27 05:35:03 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkBlock.cpp Mon Apr 27 07:13:07 2009 -0400
@@ -46,6 +46,11 @@
if (SharkTraceBytecodes)
tty->print_cr("%4d: %s", bci(), Bytecodes::name(bc()));
+ if (has_trap() && trap_bci() == bci()) {
+ do_trap(trap_request());
+ return;
+ }
+
if (UseLoopSafepoints) {
// XXX if a lcmp is followed by an if_?? then C2 maybe-inserts
// the safepoint before the lcmp rather than before the if.
@@ -1138,6 +1143,26 @@
ShouldNotCallThis();
}
+bool SharkBlock::has_trap()
+{
+ return false;
+}
+
+int SharkBlock::trap_request()
+{
+ ShouldNotCallThis();
+}
+
+int SharkBlock::trap_bci()
+{
+ ShouldNotCallThis();
+}
+
+void SharkBlock::do_trap(int trap_request)
+{
+ ShouldNotCallThis();
+}
+
Value* SharkBlock::lookup_for_ldc()
{
ShouldNotCallThis();
diff -r db61663b8232 -r 010cb02d0958 ports/hotspot/src/share/vm/shark/sharkBlock.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkBlock.hpp Mon Apr 27 05:35:03 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkBlock.hpp Mon Apr 27 07:13:07 2009 -0400
@@ -233,6 +233,13 @@
protected:
virtual void add_safepoint();
+ // Traps
+ protected:
+ virtual bool has_trap();
+ virtual int trap_request();
+ virtual int trap_bci();
+ virtual void do_trap(int trap_request);
+
// ldc*
private:
void do_ldc()
diff -r db61663b8232 -r 010cb02d0958 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Mon Apr 27 05:35:03 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Mon Apr 27 07:13:07 2009 -0400
@@ -28,19 +28,14 @@
using namespace llvm;
-int SharkTopLevelBlock::scan_for_traps()
+void SharkTopLevelBlock::scan_for_traps()
{
- // If typeflow got one then we're already done
- if (ciblock()->has_trap()) {
- return Deoptimization::make_trap_request(
- Deoptimization::Reason_unloaded,
- Deoptimization::Action_reinterpret,
- ciblock()->trap_index());
- }
+ // If typeflow found a trap then don't scan past it
+ int limit_bci = ciblock()->has_trap() ? ciblock()->trap_bci() : limit();
- // Scan the bytecode
+ // Scan the bytecode for traps that are always hit
iter()->reset_to_bci(start());
- while (iter()->next_bci() < limit()) {
+ while (iter()->next_bci() < limit_bci) {
iter()->next();
ciField *field;
@@ -62,9 +57,11 @@
// If the bytecode does not match the field then bail out to
// the interpreter to throw an IncompatibleClassChangeError
if (is_field == field->is_static()) {
- return Deoptimization::make_trap_request(
- Deoptimization::Reason_unhandled,
- Deoptimization::Action_none);
+ set_trap(
+ Deoptimization::make_trap_request(
+ Deoptimization::Reason_unhandled,
+ Deoptimization::Action_none), bci());
+ return;
}
// If this is a getfield or putfield then there won't be a
@@ -96,9 +93,11 @@
// set up otherwise.
if (bc() == Bytecodes::_invokevirtual && !method->is_final_method()) {
if (!method->holder()->is_linked()) {
- return Deoptimization::make_trap_request(
- Deoptimization::Reason_uninitialized,
- Deoptimization::Action_reinterpret);
+ set_trap(
+ Deoptimization::make_trap_request(
+ Deoptimization::Reason_uninitialized,
+ Deoptimization::Action_reinterpret), bci());
+ return;
}
break;
}
@@ -112,13 +111,24 @@
if (index != -1) {
if (!target()->holder()->is_cache_entry_resolved(
Bytes::swap_u2(index), bc())) {
- return Deoptimization::make_trap_request(
- Deoptimization::Reason_uninitialized,
- Deoptimization::Action_reinterpret);
+ set_trap(
+ Deoptimization::make_trap_request(
+ Deoptimization::Reason_uninitialized,
+ Deoptimization::Action_reinterpret), bci());
+ return;
}
}
}
- return TRAP_NO_TRAPS;
+
+ // Trap if typeflow trapped (and we didn't before)
+ if (ciblock()->has_trap()) {
+ set_trap(
+ Deoptimization::make_trap_request(
+ Deoptimization::Reason_unloaded,
+ Deoptimization::Action_reinterpret,
+ ciblock()->trap_index()), ciblock()->trap_bci());
+ return;
+ }
}
SharkState* SharkTopLevelBlock::entry_state()
@@ -163,13 +173,14 @@
if (!entered()) {
_entered = true;
+ scan_for_traps();
if (!has_trap()) {
for (int i = 0; i < num_successors(); i++) {
successor(i)->enter(this, false);
}
- for (int i = 0; i < num_exceptions(); i++) {
- exception(i)->enter(this, true);
- }
+ }
+ for (int i = 0; i < num_exceptions(); i++) {
+ exception(i)->enter(this, true);
}
}
}
@@ -215,25 +226,12 @@
{
builder()->SetInsertPoint(entry_block());
- // Handle traps
- if (has_trap()) {
- iter()->force_bci(start());
-
- current_state()->decache_for_trap();
- builder()->CreateCall2(
- SharkRuntime::uncommon_trap(),
- thread(),
- LLVMValue::jint_constant(trap_request()));
- builder()->CreateRetVoid();
- return;
- }
-
// Parse the bytecode
parse_bytecode(start(), limit());
// If this block falls through to the next then it won't have been
// terminated by a bytecode and we have to add the branch ourselves
- if (falls_through())
+ if (falls_through() && !has_trap())
do_branch(ciTypeFlow::FALL_THROUGH);
}
@@ -476,6 +474,16 @@
builder()->SetInsertPoint(safepointed);
current_state()->merge(orig_state, orig_block, safepointed_block);
+}
+
+void SharkTopLevelBlock::do_trap(int trap_request)
+{
+ current_state()->decache_for_trap();
+ builder()->CreateCall2(
+ SharkRuntime::uncommon_trap(),
+ thread(),
+ LLVMValue::jint_constant(trap_request));
+ builder()->CreateRetVoid();
}
void SharkTopLevelBlock::call_register_finalizer(Value *receiver)
diff -r db61663b8232 -r 010cb02d0958 ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp Mon Apr 27 05:35:03 2009 -0400
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.hpp Mon Apr 27 07:13:07 2009 -0400
@@ -32,8 +32,8 @@
function->thread()),
_function(function),
_ciblock(ciblock),
- _trap_request(TRAP_UNCHECKED),
_entered(false),
+ _has_trap(false),
_needs_phis(false),
_entry_state(NULL),
_entry_block(NULL) {}
@@ -106,26 +106,36 @@
// Traps
private:
- enum {
- TRAP_UNCHECKED = 232323, // > any constant pool index
- TRAP_NO_TRAPS
- };
- int _trap_request;
+ bool _has_trap;
+ int _trap_request;
+ int _trap_bci;
- public:
- int trap_request()
+ void set_trap(int trap_request, int trap_bci)
{
- if (_trap_request == TRAP_UNCHECKED)
- _trap_request = scan_for_traps();
- return _trap_request;
- }
- bool has_trap()
- {
- return trap_request() != TRAP_NO_TRAPS;
+ assert(!has_trap(), "shouldn't have");
+ _has_trap = true;
+ _trap_request = trap_request;
+ _trap_bci = trap_bci;
}
private:
- int scan_for_traps();
+ bool has_trap()
+ {
+ return _has_trap;
+ }
+ int trap_request()
+ {
+ assert(has_trap(), "should have");
+ return _trap_request;
+ }
+ int trap_bci()
+ {
+ assert(has_trap(), "should have");
+ return _trap_bci;
+ }
+
+ private:
+ void scan_for_traps();
// Entry state
private:
@@ -293,6 +303,10 @@
private:
void add_safepoint();
+ // Traps
+ private:
+ void do_trap(int trap_request);
+
// Returns
private:
void call_register_finalizer(llvm::Value* receiver);
More information about the distro-pkg-dev
mailing list