Change the profiler to be sample based (driven by preemption) (#831)

* Remove PROFILER ifdef

* Wait in microseconds

* Mark profiler tests as failing

* Sample bcps at preemption

* Avoid spurious wakeups of preemption thread

* Encode method offset in backwards branches

* Remove wrong push

* Cleanup

* Fix profiler tests

* Report ticks, not bytecodes

* Update bytecode printing

src/bytecodes.h

@@ -52,7 +52,6 @@ namespace toit {
   FORMAT(OP_BC, 2)               \
   FORMAT(OP_BG, 2)               \
   FORMAT(OP_BF, 2)               \
-  FORMAT(OP_BB, 2)               \
   FORMAT(OP_BCI, 2)              \
   FORMAT(OP_BII, 2)              \
   FORMAT(OP_BLC, 2)              \
@@ -74,11 +73,10 @@ namespace toit {
   FORMAT(OP_SS_SO, 5)            \
   FORMAT(OP_SCI, 3)              \
   FORMAT(OP_SII, 3)              \
-  FORMAT(OP_SB, 3)               \
+  FORMAT(OP_SB_SB, 5)            \
   FORMAT(OP_SU_SU, 5)            \
 
 
-
 // Format Toit bytecodes
 enum BytecodeFormat {
 #define THE_FORMAT(format, length) format,
@@ -180,12 +178,9 @@ enum BytecodeFormat {
   BYTECODE(BRANCH,                     3, OP_SF, "branch")                     \
   BYTECODE(BRANCH_IF_TRUE,             3, OP_SF, "branch if true")             \
   BYTECODE(BRANCH_IF_FALSE,            3, OP_SF, "branch if false")            \
-  BYTECODE(BRANCH_BACK,                2, OP_BB, "branch back")                \
-  BYTECODE(BRANCH_BACK_WIDE,           3, OP_SB, "branch back wide")           \
-  BYTECODE(BRANCH_BACK_IF_TRUE,        2, OP_BB, "branch back if true")        \
-  BYTECODE(BRANCH_BACK_IF_TRUE_WIDE,   3, OP_SB, "branch back if true wide")   \
-  BYTECODE(BRANCH_BACK_IF_FALSE,       2, OP_BB, "branch back if false")       \
-  BYTECODE(BRANCH_BACK_IF_FALSE_WIDE,  3, OP_SB, "branch back if false wide")  \
+  BYTECODE(BRANCH_BACK,                5, OP_SB_SB, "branch back")             \
+  BYTECODE(BRANCH_BACK_IF_TRUE,        5, OP_SB_SB, "branch back if true")     \
+  BYTECODE(BRANCH_BACK_IF_FALSE,       5, OP_SB_SB, "branch back if false")    \
   BYTECODE(PRIMITIVE,                  4, OP_BU_SU, "invoke primitive")        \
   BYTECODE(THROW,                      2, OP_BU, "throw")                      \
   BYTECODE(RETURN,                     3, OP_BS_BU, "return")                  \

src/compiler/emitter.cc

@@ -454,7 +454,9 @@ void Emitter::branch(Condition condition, Label* label) {
   if (label->is_bound()) {
     int offset = -(label->position() - position);
     ASSERT(offset >= 0);
-    emit_possibly_wide(op, offset);
+    emit_opcode(op);
+    emit_uint16(offset);
+    emit_uint16(position);
   } else {
     label->use(position, height());
     emit_opcode(op);

src/encoder.cc

@@ -264,12 +264,10 @@ bool ProgramOrientedEncoder::encode_error(Object* type, const char* message, Sta
   return !buffer()->has_overflow();
 }
 
-#ifdef PROFILER
 bool ProgramOrientedEncoder::encode_profile(Profiler* profiler, String* title, int cutoff) {
   profiler->encode_on(this, title, cutoff);
   return !buffer()->has_overflow();
 }
-#endif
 
 void Encoder::write_byte(uint8 c) {
   _buffer->put_byte(c);

src/encoder.h

@@ -133,9 +133,7 @@ class ProgramOrientedEncoder : public Encoder {
   bool encode_error(Object* type, Object* message, Stack* stack);
   bool encode_error(Object* type, const char* message, Stack* stack);
 
-#ifdef PROFILER
   bool encode_profile(Profiler* profile, String* title, int cutoff);
-#endif
 
   Program* program() { return _program; }
 

src/event_sources/timer.cc

@@ -114,9 +114,11 @@ void TimerEventSource::entry() {
         break;
       }
     }
-
-    int delay_ms = (delay_us + 1000 - 1) / 1000;  // Ceiling division.
-    OS::wait(_timer_changed, delay_ms);
+    if (delay_us > 0) {
+      OS::wait_us(_timer_changed, delay_us);
+    } else {
+      OS::wait(_timer_changed);
+    }
   }
 }
 

src/interpreter.cc

@@ -38,9 +38,6 @@ Interpreter::Interpreter()
     , _sp(null)
     , _try_sp(null)
     , _watermark(null) {
-#ifdef PROFILER
-  _is_profiler_active = false;
-#endif
 }
 
 void Interpreter::activate(Process* process) {
@@ -55,20 +52,6 @@ void Interpreter::preempt() {
   _watermark = PREEMPTION_MARKER;
 }
 
-#ifdef PROFILER
-void Interpreter::profile_register_method(Method method) {
-  int absolute_bci = process()->program()->absolute_bci_from_bcp(method.header_bcp());
-  Profiler* profiler = process()->profiler();
-  if (profiler != null) profiler->register_method(absolute_bci);
-}
-
-void Interpreter::profile_increment(uint8* bcp) {
-  int absolute_bci = process()->program()->absolute_bci_from_bcp(bcp);
-  Profiler* profiler = process()->profiler();
-  if (profiler != null) profiler->increment(absolute_bci);
-}
-#endif
-
 Method Interpreter::lookup_entry() {
   Method result = _process->entry();
   if (!result.is_valid()) FATAL("Cannot locate entry method for interpreter");
@@ -79,9 +62,6 @@ Object** Interpreter::load_stack() {
   Stack* stack = _process->task()->stack();
   GcMetadata::insert_into_remembered_set(stack);
   stack->transfer_to_interpreter(this);
-#ifdef PROFILER
-  set_profiler_state();
-#endif
   Object** watermark = _watermark;
   Object** new_watermark = _limit + RESERVED_STACK_FOR_STACK_OVERFLOWS;
   while (true) {
@@ -107,13 +87,6 @@ void Interpreter::store_stack(Object** sp) {
   }
 }
 
-#ifdef PROFILER
-void Interpreter::set_profiler_state() {
-  Profiler* profiler = process()->profiler();
-  _is_profiler_active = profiler != null && profiler->should_profile_task(process()->task()->id());
-}
-#endif
-
 void Interpreter::prepare_task(Method entry, Instance* code) {
   push(code);
   static_assert(FRAME_SIZE == 2, "Unexpected frame size");

src/interpreter.h

@@ -113,6 +113,7 @@ class Interpreter {
   void prepare_task(Method entry, Instance* code);
 
   void preempt();
+  uint8* preemption_method_header_bcp() const { return _preemption_method_header_bcp; }
 
  private:
   Object** const PREEMPTION_MARKER = reinterpret_cast<Object**>(UINTPTR_MAX);
@@ -127,16 +128,12 @@ class Interpreter {
   // Stack overflow handling.
   std::atomic<Object**> _watermark;
 
+  // Preemption method.
+  uint8* _preemption_method_header_bcp;
+
   void trace(uint8* bcp);
   Method lookup_entry();
 
-#ifdef PROFILER
-  bool _is_profiler_active;
-  void profile_register_method(Method method);
-  void profile_increment(uint8* bcp);
-  void set_profiler_state();
-#endif
-
   enum OverflowState {
     OVERFLOW_RESUME,
     OVERFLOW_PREEMPT,

src/interpreter_run.cc

@@ -87,15 +87,8 @@ Method Program::find_method(Object* receiver, int offset) {
 
 // OPCODE_TRACE is only called from within Interpreter::run which gives access to:
 //   uint8* bcp;
-//   uword index;
-#ifdef PROFILER
-#define OPCODE_TRACE()                                         \
-  if (_is_profiler_active) profile_increment(bcp);             \
+#define OPCODE_TRACE() \
   if (Flags::trace) trace(bcp);
-#else
-#define OPCODE_TRACE()                                         \
-  if (Flags::trace) trace(bcp);
-#endif
 
 // Dispatching helper macros.
 #define DISPATCH(n)                                                                \
@@ -145,13 +138,6 @@ Method Program::find_method(Object* receiver, int offset) {
 #define B_ARG1(name) uint8 name = bcp[1];
 #define S_ARG1(name) uint16 name = Utils::read_unaligned_uint16(bcp + 1);
 
-#ifdef PROFILER
-#define REGISTER_METHOD(target)                             \
-  if (_is_profiler_active) profile_register_method(target);
-#else
-#define REGISTER_METHOD(target)
-#endif
-
 // CHECK_STACK_OVERFLOW checks if there is enough stack space to call
 // the given target method.
 #define CHECK_STACK_OVERFLOW(target)                                  \
@@ -162,6 +148,7 @@ Method Program::find_method(Object* receiver, int offset) {
       case OVERFLOW_RESUME:                                           \
         break;                                                        \
       case OVERFLOW_PREEMPT:                                          \
+        _preemption_method_header_bcp = target.header_bcp();          \
         static_assert(FRAME_SIZE == 2, "Unexpected frame size");      \
         PUSH(reinterpret_cast<Object*>(target.entry()));              \
         PUSH(program->frame_marker());                                \
@@ -173,13 +160,14 @@ Method Program::find_method(Object* receiver, int offset) {
   }
 
 // CHECK_PREEMPT checks for preemption and watchdog interrupts.
-#define CHECK_PREEMPT()                                               \
+#define CHECK_PREEMPT(entry)                                          \
   if (_watermark == PREEMPTION_MARKER) {                              \
     OverflowState state;                                              \
     sp = handle_preempt(sp, &state);                                  \
     if (state == OVERFLOW_EXCEPTION) {                                \
       goto THROW_IMPLEMENTATION;                                      \
     }                                                                 \
+    _preemption_method_header_bcp = Method::header_from_entry(entry); \
     static_assert(FRAME_SIZE == 2, "Unexpected frame size");          \
     PUSH(reinterpret_cast<Object*>(bcp));                             \
     PUSH(program->frame_marker());                                    \
@@ -191,7 +179,6 @@ Method Program::find_method(Object* receiver, int offset) {
   static_assert(FRAME_SIZE == 2, "Unexpected frame size");            \
   PUSH(reinterpret_cast<Object*>(return_address));                    \
   PUSH(program->frame_marker());                                      \
-  REGISTER_METHOD(target);                                            \
   CHECK_STACK_OVERFLOW(target)                                        \
   bcp = target.entry();                                               \
   DISPATCH(0)
@@ -270,6 +257,7 @@ Interpreter::Result Interpreter::run() {
 #undef LABEL
 
   // Interpretation state.
+  _preemption_method_header_bcp = null;
   Object** sp = load_stack();
   Program* program = _process->program();
   uword _index_ = 0;
@@ -279,11 +267,6 @@ Interpreter::Result Interpreter::run() {
     ASSERT(frame_marker == program->frame_marker());
   }
   uint8* bcp = reinterpret_cast<uint8*>(POP());
-
-#ifdef PROFILER
-  set_profiler_state();
-#endif
-
   DISPATCH(0);
 
   OPCODE_BEGIN_WITH_WIDE(LOAD_LOCAL, stack_offset);
@@ -624,7 +607,7 @@ Interpreter::Result Interpreter::run() {
     } else {
       DROP(extra);
     }
-    CALL_METHOD(target, INVOKE_BLOCK_LENGTH)
+    CALL_METHOD(target, INVOKE_BLOCK_LENGTH);
   OPCODE_END();
 
   OPCODE_BEGIN(INVOKE_INITIALIZER_TAIL);
@@ -900,24 +883,27 @@ Interpreter::Result Interpreter::run() {
     }
   OPCODE_END();
 
-  OPCODE_BEGIN_WITH_WIDE(BRANCH_BACK, relative_offset);
-    bcp -= relative_offset;
-    CHECK_PREEMPT();
+  OPCODE_BEGIN(BRANCH_BACK);
+    uint8* entry = bcp - Utils::read_unaligned_uint16(bcp + 3);
+    bcp -= Utils::read_unaligned_uint16(bcp + 1);
+    CHECK_PREEMPT(entry);
     DISPATCH(0);
   OPCODE_END();
 
-  OPCODE_BEGIN_WITH_WIDE(BRANCH_BACK_IF_TRUE, relative_offset);
+  OPCODE_BEGIN(BRANCH_BACK_IF_TRUE);
     if (is_true_value(program, POP())) {
-      bcp -= relative_offset;
-      CHECK_PREEMPT();
+      uint8* entry = bcp - Utils::read_unaligned_uint16(bcp + 3);
+      bcp -= Utils::read_unaligned_uint16(bcp + 1);
+      CHECK_PREEMPT(entry);
       DISPATCH(0);
     }
   OPCODE_END();
 
-  OPCODE_BEGIN_WITH_WIDE(BRANCH_BACK_IF_FALSE, relative_offset);
+  OPCODE_BEGIN(BRANCH_BACK_IF_FALSE);
     if (!is_true_value(program, POP())) {
-      bcp -= relative_offset;
-      CHECK_PREEMPT();
+      uint8* entry = bcp - Utils::read_unaligned_uint16(bcp + 3);
+      bcp -= Utils::read_unaligned_uint16(bcp + 1);
+      CHECK_PREEMPT(entry);
       DISPATCH(0);
     }
   OPCODE_END();

src/objects.cc

@@ -219,6 +219,17 @@ void Array::roots_do(RootCallback* cb) {
   cb->do_roots(_root_at(_offset_from(0)), length());
 }
 
+int Stack::absolute_bci_at_preemption(Program* program) {
+  // Check that the stack has both words.
+  if (_stack_sp_addr() + 1 >= _stack_base_addr()) return -1;
+  // Check that the frame marker is correct.
+  if (at(0) != program->frame_marker()) return -1;
+  // Get the bytecode pointer and convert it to an index.
+  uint8* bcp = reinterpret_cast<uint8*>(at(1));
+  if (!program->bytecodes.is_inside(bcp)) return -1;
+  return program->absolute_bci_from_bcp(bcp);
+}
+
 void Stack::roots_do(Program* program, RootCallback* cb) {
   int top = this->top();
   // Skip over pointers into the bytecodes.

src/objects.h

@@ -683,6 +683,8 @@ class Stack : public HeapObject {
   int top() { return _word_at(TOP_OFFSET); }
   int try_top() { return _word_at(TRY_TOP_OFFSET); }
 
+  int absolute_bci_at_preemption(Program* program);
+
   void transfer_to_interpreter(Interpreter* interpreter);
   void transfer_from_interpreter(Interpreter* interpreter);
 
@@ -720,11 +722,13 @@ class Stack : public HeapObject {
   void _set_length(int value) { _word_at_put(LENGTH_OFFSET, value); }
   void _set_top(int value) { _word_at_put(TOP_OFFSET, value); }
   void _set_try_top(int value) { _word_at_put(TRY_TOP_OFFSET, value); }
+
   void _initialize(int length) {
     _set_length(length);
     _set_top(length);
     _set_try_top(length);
   }
+
   Object** _stack_base_addr() { return reinterpret_cast<Object**>(_raw_at(_array_offset_from(length()))); }
   Object** _stack_limit_addr() { return reinterpret_cast<Object**>(_raw_at(_array_offset_from(0))); }
   Object** _stack_sp_addr() { return reinterpret_cast<Object**>(_raw_at(_array_offset_from(top()))); }
@@ -748,8 +752,8 @@ class Stack : public HeapObject {
   }
 
   uword* _array_address(int index) { return _raw_at(_array_offset_from(index)); }
-
   static int _array_offset_from(int index) { return HEADER_SIZE + index  * WORD_SIZE; }
+
   friend class ObjectHeap;
   friend class ProgramHeap;
 };
@@ -1073,6 +1077,8 @@ class Method {
   uint8* bcp_from_bci(int bci) const { return &_bytes[ENTRY_OFFSET + bci]; }
   uint8* header_bcp() const { return _bytes; }
 
+  static uint8* header_from_entry(uint8* entry) { return entry - ENTRY_OFFSET; }
+
  private: // Friend access for ProgramBuilder.
   void _initialize_block(int arity, List<uint8> bytecodes, int max_height) {
     _initialize(BLOCK, 0, arity, bytecodes, max_height);

src/os.h

@@ -187,7 +187,7 @@ class OS {
   static ConditionVariable* allocate_condition_variable(Mutex* mutex);
   static void wait(ConditionVariable* condition_variable);
   // Returns false if a timeout occurs.
-  static bool wait(ConditionVariable* condition_variable, int timeout_in_ms);
+  static bool wait_us(ConditionVariable* condition_variable, int64 us);
   static void signal(ConditionVariable* condition_variable);
   static void signal_all(ConditionVariable* condition_variable);
   static void dispose(ConditionVariable* condition_variable);