Merge pull request #872 from kmod/perf2

Rewriter "aggressiveness" and backoff

src/asm_writing/icinfo.cpp

@@ -30,7 +30,6 @@ namespace pyston {
 
 using namespace pyston::assembler;
 
-#define MEGAMORPHIC_THRESHOLD 100
 #define MAX_RETRY_BACKOFF 1024
 
 // TODO not right place for this...
@@ -121,7 +120,7 @@ void ICSlotRewrite::commit(CommitHook* hook) {
 
     ic->times_rewritten++;
 
-    if (ic->times_rewritten == MEGAMORPHIC_THRESHOLD) {
+    if (ic->times_rewritten == IC_MEGAMORPHIC_THRESHOLD) {
         static StatCounter megamorphic_ics("megamorphic_ics");
         megamorphic_ics.log();
     }
@@ -298,6 +297,6 @@ bool ICInfo::shouldAttempt() {
 }
 
 bool ICInfo::isMegamorphic() {
-    return times_rewritten >= MEGAMORPHIC_THRESHOLD;
+    return times_rewritten >= IC_MEGAMORPHIC_THRESHOLD;
 }
 }

src/asm_writing/icinfo.h

@@ -33,6 +33,7 @@ class ICInfo;
 class ICInvalidator;
 
 #define IC_INVALDITION_HEADER_SIZE 6
+#define IC_MEGAMORPHIC_THRESHOLD 100
 
 struct ICSlotInfo {
 public:
@@ -134,6 +135,10 @@ public:
     bool shouldAttempt();
     bool isMegamorphic();
 
+    // For use of the rewriter for computing aggressiveness:
+    int percentMegamorphic() const { return times_rewritten * 100 / IC_MEGAMORPHIC_THRESHOLD; }
+    int percentBackedoff() const { return retry_backoff; }
+
     friend class ICSlotRewrite;
 };
 

src/asm_writing/rewriter.h

@@ -589,6 +589,17 @@ public:
 
     static bool isLargeConstant(int64_t val) { return (val < (-1L << 31) || val >= (1L << 31) - 1); }
 
+    // The "aggressiveness" with which we should try to do this rewrite.  It starts high, and decreases over time.
+    // The values are nominally in the range 0-100, with 0 being no aggressiveness and 100 being fully aggressive,
+    // but callers should be prepared to receive values from a larger range.
+    //
+    // It would be nice to have this be stateful so that we could support things like "Lower the aggressiveness for
+    // this sub-call and then increase it back afterwards".
+    int aggressiveness() {
+        const ICInfo* ic = rewrite->getICInfo();
+        return 100 - ic->percentBackedoff() - ic->percentMegamorphic();
+    }
+
     friend class RewriterVar;
 };
 

src/capi/typeobject.cpp

@@ -1044,8 +1044,10 @@ Box* slotTpGetattrHookInternal(Box* self, BoxedString* name, GetattrRewriteArgs*
 
             if (!grewrite_args.out_success)
                 rewrite_args = NULL;
-            else if (res)
+            else if (res) {
                 rewrite_args->out_rtn = grewrite_args.out_rtn;
+                rewrite_args->out_return_convention = grewrite_args.out_return_convention;
+            }
         } else {
             try {
                 res = getattrInternalGeneric(self, name, NULL, false, false, NULL, NULL);
@@ -1110,8 +1112,10 @@ Box* slotTpGetattrHookInternal(Box* self, BoxedString* name, GetattrRewriteArgs*
 
         if (!crewrite_args.out_success)
             rewrite_args = NULL;
-        else
+        else {
             rewrite_args->out_rtn = crewrite_args.out_rtn;
+            rewrite_args->out_return_convention = GetattrRewriteArgs::VALID_RETURN;
+        }
     } else {
         // TODO: we already fetched the getattr attribute, it would be faster to call it rather than do
         // a second callattr.  My guess though is that the gains would be small, so I would prefer to keep

src/codegen/type_recording.cpp

@@ -33,6 +33,11 @@ Box* recordType(TypeRecorder* self, Box* obj) {
     // The baseline JIT directly generates machine code for this function inside JitFragmentWriter::_emitRecordType.
     // When changing this function one has to also change the bjit code.
 
+    if (!obj) {
+        assert(PyErr_Occurred());
+        return obj;
+    }
+
     BoxedClass* cls = obj->cls;
     if (cls != self->last_seen) {
         self->last_seen = cls;

src/runtime/list.cpp

@@ -986,7 +986,7 @@ Box* listNew(BoxedClass* cls, Box* container) {
 Box* listInit(BoxedList* self, Box* container) {
     assert(PyList_Check(self));
 
-    if (container != None) {
+    if (container) {
         listIAdd(self, container);
     }
 
@@ -1243,7 +1243,7 @@ void setupList() {
     list_cls->giveAttr("__new__",
                        new BoxedFunction(boxRTFunction((void*)listNew, UNKNOWN, 2, 1, false, false), { None }));
     list_cls->giveAttr("__init__",
-                       new BoxedFunction(boxRTFunction((void*)listInit, UNKNOWN, 2, 1, false, false), { None }));
+                       new BoxedFunction(boxRTFunction((void*)listInit, UNKNOWN, 2, 1, false, false), { NULL }));
 
     list_cls->giveAttr("count", new BoxedFunction(boxRTFunction((void*)listCount, BOXED_INT, 2)));
     list_cls->giveAttr(

src/runtime/rewrite_args.h

@@ -30,6 +30,28 @@ struct GetattrRewriteArgs {
     bool obj_hcls_guarded;
     bool obj_shape_guarded; // "shape" as in whether there are hcls attrs and where they live
 
+    // We have have a couple different conventions for returning values from getattr-like functions.
+    // For normal code we have just two conventions:
+    // - the "normal" convention is that signalling the lack of an attribute is handled by throwing
+    //   an exception (via either CAPI or C++ means).  This is the only convention that CPython has.
+    // - our fast "no exception" convention which will return NULL and not throw an exception, not
+    //   even a CAPI exception.
+    //
+    // For the rewriter, there are three cases:
+    // - we will obey the "normal" convention (VALID_RETURN)
+    // - we will never have anything to return and there will be no exception (NO_RETURN)
+    // - we don't know which of the above two will happen (NOEXC_POSSIBLE)
+    //
+    // UNSPECIFIED is used as an invalid-default to make sure that we don't implicitly assume one
+    // of the cases when the callee didn't explicitly signal one.
+    //
+    enum ReturnConvention {
+        UNSPECIFIED,
+        NO_RETURN,
+        NOEXC_POSSIBLE,
+        VALID_RETURN,
+    } out_return_convention;
+
     GetattrRewriteArgs(Rewriter* rewriter, RewriterVar* obj, Location destination)
         : rewriter(rewriter),
           obj(obj),
@@ -37,7 +59,8 @@ struct GetattrRewriteArgs {
           out_success(false),
           out_rtn(NULL),
           obj_hcls_guarded(false),
-          obj_shape_guarded(false) {}
+          obj_shape_guarded(false),
+          out_return_convention(UNSPECIFIED) {}
 };
 
 struct SetattrRewriteArgs {

src/runtime/types.h

@@ -557,6 +557,18 @@ public:
             assert(gc::isValidGCObject(rtn->elts[i]));
         return rtn;
     }
+    static BoxedTuple* create6(Box* elt0, Box* elt1, Box* elt2, Box* elt3, Box* elt4, Box* elt5) {
+        BoxedTuple* rtn = new (6) BoxedTuple();
+        rtn->elts[0] = elt0;
+        rtn->elts[1] = elt1;
+        rtn->elts[2] = elt2;
+        rtn->elts[3] = elt3;
+        rtn->elts[4] = elt4;
+        rtn->elts[5] = elt5;
+        for (int i = 0; i < rtn->size(); i++)
+            assert(gc::isValidGCObject(rtn->elts[i]));
+        return rtn;
+    }
     static BoxedTuple* create(std::initializer_list<Box*> members) {
         auto rtn = new (members.size()) BoxedTuple(members);