address code review concerns

rsqueakvm/interpreter.py

@@ -130,6 +130,16 @@ class Optargs(object):
     _attrs_ = ["max_squeak_unroll_count", "squeak_unroll_trace_limit"]
     _immutable_fields_ = ["max_squeak_unroll_count", "squeak_unroll_trace_limit"]
     def __init__(self):
+        """
+        - max_squeak_unroll_count: How many times to unroll a loop before
+          JIT'ing. The default (2) was chosen because it "felt right".
+        - squeak_unroll_trace_limit: After how many operations we should never
+          unroll. This tries to avoid trace_too_long aborts, and should kind of
+          be synchronized with the default trace_limit. Imagine you have a long
+          loop, and you unroll it twice, then we're at 86k ops with the default
+          of 32k ops. This is alreay quite a long trace, we don't want to blow
+          it up too much.
+        """
         self.max_squeak_unroll_count = 2
         self.squeak_unroll_trace_limit = 32000
 

rsqueakvm/main.py

@@ -68,6 +68,10 @@ def _usage(argv):
                                headless mode, result printed.
             -rr <code>       - Code will be compiled and executed in
                                headless mode, twice, the second result printed.
+                               This is a workaround for making jittests and
+                               benchmarking easier, because do-its are not JIT'ed
+                               right now, due to the dynamic frame size
+                               calculation.
             -m|--method <selector>
                              - Selector will be sent to nil in
                                headless mode, result printed.

rsqueakvm/model/compiled_methods.py

@@ -392,8 +392,7 @@ def bytecode_string(self, markBytecode=0):
         from rsqueakvm.interpreter_bytecodes import BYTECODE_TABLE
         retval = "Bytecode:------------"
         j = 0
-        while j < len(self.bytes):
-            i = self.bytes[idx]
+        for i in self.bytes:
             retval += '\n'
             retval += '->' if j + 1 is markBytecode else '  '
             retval += ('%0.2i: 0x%0.2x(%0.3i) ' % (j + 1, ord(i), ord(i))) + BYTECODE_TABLE[ord(i)].__name__
@@ -408,10 +407,10 @@ def as_string(self, markBytecode=0):
 
     def guess_containing_classname(self):
         w_class = self.compiled_in()
-        if w_class:
-            # Not pretty to steal the space from another object.
-            return w_class.as_class_get_shadow(w_class.space()).getname()
-        return "? (no compiledin-info)"
+        if w_class is None:
+            return "? (no compiledin-info)"
+        # Not pretty to steal the space from another object.
+        return w_class.as_class_get_shadow(w_class.space()).getname()
 
     def get_identifier_string(self):
         return "%s >> #%s" % (self.guess_containing_classname(), self.lookup_selector)

rsqueakvm/model/pointers.py

@@ -234,9 +234,14 @@ def clone(self, space):
         return w_result
 
 
-# when changing this constant, also change the read and __write__ methods in
-# IntMapStorageNode in storage.py (or generalize those methods to compile with
-# getattrs)
+# When changing these constants, also change the read and __write__ methods in
+# IntMapStorageNode in storage.py (or generalize those methods, while you're at
+# it). The numbers for these fields where chosen semi-randomly. Ranges (which
+# are used in iterations when doing ((1 to: 2) do: []) have 3 integer fields, so
+# three seemed like a good number here. Collections often have two integer
+# fields and one or two pointer objects. Rectangles have two Points, Points two
+# integer fields. I didn't want to go too large to not waste so much
+# memory. More measurements would probably be required.
 _NUMBER_OF_INT_FIELDS = 3
 _NUMBER_OF_INLINE_FIELDS = 2
 class W_FixedPointersObject(W_PointersObject):

rsqueakvm/primitives/__init__.py

@@ -20,10 +20,9 @@ def assert_class(interp, w_obj, w_class):
 def assert_valid_index(space, n0, w_obj):
     if not int_between(0, n0, w_obj.varsize()):
         raise PrimitiveFailedError()
-    # return the index, since from here on the annotator knows that
-    # n0 cannot be negative
-    assert n0 >= 0
-    return n0
+    else: # n0 cannot be negative due to the if condition. help the annotator.
+        assert n0 >= 0
+        return n0
 
 def assert_valid_inst_index(space, n0, w_obj):
     if not int_between(0, n0, w_obj.size()):

rsqueakvm/storage_contexts.py

@@ -168,12 +168,13 @@ def is_privileged_index(self, n0):
 
     def get_extra_data(self):
         extra_data = self.extra_data_or_blockmethod
-        if extra_data is None or not isinstance(extra_data, ExtraContextAttributes):
+        if isinstance(extra_data, ExtraContextAttributes):
+            return extra_data
+        else:
             new_extra_data = ExtraContextAttributes()
             new_extra_data.blockmethod = extra_data
-            extra_data = self.extra_data_or_blockmethod = new_extra_data
-        assert isinstance(extra_data, ExtraContextAttributes)
-        return extra_data
+            self.extra_data_or_blockmethod = new_extra_data
+            return new_extra_data
 
     def extra_data(self):
         extra_data = self.extra_data_or_blockmethod
@@ -885,10 +886,9 @@ def blockmethod(self):
         w_bm = self.extra_data_or_blockmethod
         if isinstance(w_bm, ExtraContextAttributes):
             w_bm = w_bm.blockmethod
-        if w_bm is None:
-            return None
-        assert isinstance(w_bm, W_CompiledMethod)
-        return w_bm
+        if isinstance(w_bm, W_CompiledMethod):
+            return w_bm
+        return None
 
     def set_blockmethod(self, w_obj):
         assert isinstance(w_obj, W_CompiledMethod)