From bc0162ccb8522a32a11eaee5ce0837b67577412e Mon Sep 17 00:00:00 2001 From: Tim Felgentreff Date: Thu, 23 Mar 2017 14:56:45 +0100 Subject: [PATCH] address code review concerns --- rsqueakvm/interpreter.py | 10 ++++++++++ rsqueakvm/main.py | 4 ++++ rsqueakvm/model/compiled_methods.py | 11 +++++------ rsqueakvm/model/pointers.py | 11 ++++++++--- rsqueakvm/primitives/__init__.py | 7 +++---- rsqueakvm/storage_contexts.py | 16 ++++++++-------- 6 files changed, 38 insertions(+), 21 deletions(-) diff --git a/rsqueakvm/interpreter.py b/rsqueakvm/interpreter.py index cc8b6826..0ec4e8bf 100644 --- a/rsqueakvm/interpreter.py +++ b/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 diff --git a/rsqueakvm/main.py b/rsqueakvm/main.py index 1eee5c64..b1524d0f 100644 --- a/rsqueakvm/main.py +++ b/rsqueakvm/main.py @@ -68,6 +68,10 @@ def _usage(argv): headless mode, result printed. -rr - 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 will be sent to nil in headless mode, result printed. diff --git a/rsqueakvm/model/compiled_methods.py b/rsqueakvm/model/compiled_methods.py index 8d302cda..5134b340 100644 --- a/rsqueakvm/model/compiled_methods.py +++ b/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) diff --git a/rsqueakvm/model/pointers.py b/rsqueakvm/model/pointers.py index 533621d7..814387d6 100644 --- a/rsqueakvm/model/pointers.py +++ b/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): diff --git a/rsqueakvm/primitives/__init__.py b/rsqueakvm/primitives/__init__.py index 970a8334..99e24734 100644 --- a/rsqueakvm/primitives/__init__.py +++ b/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()): diff --git a/rsqueakvm/storage_contexts.py b/rsqueakvm/storage_contexts.py index 2d57bba9..759576cc 100644 --- a/rsqueakvm/storage_contexts.py +++ b/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)