[WIP] Refactor Stack to eliminate mix types

.circleci/config.yml

@@ -133,6 +133,12 @@ jobs:
       - image: circleci/python:3.6
         environment:
           TOXENV: py36-benchmark
+  py36-stack-benchmark:
+    <<: *common
+    docker:
+        - image: circleci/python:3.6
+          environment:
+            TOXENV: py36-stack-benchmark
   py36-native-blockchain-byzantium:
     <<: *common
     docker:
@@ -237,6 +243,7 @@ workflows:
       - py36-native-blockchain-transition
       - py36-vm
       - py36-benchmark
+      - py36-stack-benchmark
       - py36-core
       - py36-transactions
       - py36-database

eth/vm/computation.py

@@ -19,11 +19,15 @@
 )
 from eth_utils import (
     encode_hex,
+    int_to_big_endian,
 )
 
 from eth.constants import (
+    ANY,
+    BYTES,
     GAS_MEMORY,
     GAS_MEMORY_QUADRATIC_DENOMINATOR,
+    UINT256,
 )
 from eth.exceptions import (
     Halt,
@@ -319,7 +323,41 @@ def stack_pop(self, num_items: int=1, type_hint: str=None) -> Any:
         Raise `eth.exceptions.InsufficientStack` if there are not enough items on
         the stack.
         """
-        return self._stack.pop(num_items, type_hint)
+        if num_items == 1:
+            stack_item = self._stack.pop()
+            if type_hint == UINT256 or type_hint == ANY:
+                return stack_item
+            elif type_hint == BYTES:
+                return int_to_big_endian(stack_item)
+
+        else:
+            popped_items = tuple(self._stack.pop_n(num_items))
+            if type_hint == UINT256 or type_hint == ANY:
+                return popped_items
+            elif type_hint == BYTES:
+                return tuple(int_to_big_endian(item) for item in popped_items)
+                # for item in popped_items:
+                #     yield int_to_big_endian(item)
+
+        # elif num_items == 2:
+        #     stack_item1, stack_item2 = self._stack.pop2()
+        #     if type_hint == UINT256 or type_hint == ANY:
+        #         return stack_item1, stack_item2
+        #     elif type_hint == BYTES:
+        #         return (int_to_big_endian(stack_item1), int_to_big_endian(stack_item2))
+        # elif num_items == 3:
+        #     stack_item1, stack_item2, stack_item3 = self._stack.pop3()
+        #     if type_hint == UINT256 or type_hint == ANY:
+        #         return stack_item1, stack_item2, stack_item3
+        #     elif type_hint == BYTES:
+        #         return (
+        #             int_to_big_endian(stack_item1),
+        #             int_to_big_endian(stack_item2),
+        #             int_to_big_endian(stack_item3),
+        #         )
+        # else:
+        #     # TODO: Replace with suitable customized Exception
+        #     raise Exception("Cannot pop more than 3 elements from stack")
 
     def stack_push(self, value: Union[int, bytes]) -> None:
         """

eth/vm/stack.py

@@ -29,6 +29,119 @@ class Stack(object):
     __slots__ = ['values']
     logger = logging.getLogger('eth.vm.stack.Stack')
 
+    def __init__(self) -> None:
+        self.values = []  # type: List[int]
+
+    def __len__(self) -> int:
+        return len(self.values)
+
+    def push(self, value: Union[int, bytes]) -> None:
+        """
+        Push an item onto the stack.
+        """
+        if len(self.values) > 1023:
+            raise FullStack('Stack limit reached')
+
+        validate_stack_item(value)
+
+        if isinstance(value, int):
+            stack_value = value
+        elif isinstance(value, bytes):
+            stack_value = big_endian_to_int(value)
+        else:
+            raise Exception(
+                "Stack supports only Int or Byte objects, got {} type object".format(type(value))
+            )
+
+        self.values.append(stack_value)
+
+    # def pop(self,
+    #         num_items: int,
+    #         type_hint: str) -> Union[int, bytes, Tuple[Union[int, bytes], ...]]:
+    #     """
+    #     Pop an item off the stack.
+    #
+    #     Note: This function is optimized for speed over readability.
+    #     """
+    #     try:
+    #         if num_items == 1:
+    #             return next(self._pop(num_items, type_hint))
+    #         else:
+    #             return tuple(self._pop(num_items, type_hint))
+    #     except IndexError:
+    #         raise InsufficientStack("No stack items")
+    #
+    # def _pop(self, num_items: int, type_hint: str) -> Generator[Union[int, bytes], None, None]:
+    #     for _ in range(num_items):
+    #         if type_hint == constants.UINT256:
+    #             value = self.values.pop()
+    #             if isinstance(value, int):
+    #                 yield value
+    #             else:
+    #                 yield big_endian_to_int(value)
+    #         elif type_hint == constants.BYTES:
+    #             value = self.values.pop()
+    #             if isinstance(value, bytes):
+    #                 yield value
+    #             else:
+    #                 yield int_to_big_endian(value)
+    #         elif type_hint == constants.ANY:
+    #             yield self.values.pop()
+    #         else:
+    #             raise TypeError(
+    #                 "Unknown type_hint: {0}.  Must be one of {1}".format(
+    #                     type_hint,
+    #                     ", ".join((constants.UINT256, constants.BYTES)),
+    #                 )
+    #             )
+
+    def pop(self) -> int:
+        try:
+            return self.values.pop()
+        except IndexError:
+            raise InsufficientStack("No stack items")
+
+    def pop_n(self, num_items: int) -> Tuple[int, ...]:
+        try:
+            return tuple(self._pop_n(num_items))
+        except IndexError:
+            raise InsufficientStack("No stack items")
+
+    def _pop_n(self, num_items: int) -> Tuple[int, ...]:
+        for _ in range(num_items):
+            try:
+                yield self.values.pop()
+            except IndexError:
+                raise InsufficientStack("No stack items")
+
+    def swap(self, position: int) -> None:
+        """
+        Perform a SWAP operation on the stack.
+        """
+        idx = -1 * position - 1
+        try:
+            self.values[-1], self.values[idx] = self.values[idx], self.values[-1]
+        except IndexError:
+            raise InsufficientStack("Insufficient stack items for SWAP{0}".format(position))
+
+    def dup(self, position: int) -> None:
+        """
+        Perform a DUP operation on the stack.
+        """
+        idx = -1 * position
+        try:
+            self.push(self.values[idx])
+        except IndexError:
+            raise InsufficientStack("Insufficient stack items for DUP{0}".format(position))
+
+
+class Stack_Old(object):
+    """
+    VM Stack
+    """
+    __slots__ = ['values']
+    logger = logging.getLogger('eth.vm.stack.Stack')
+
     def __init__(self) -> None:
         self.values = []  # type: List[Union[int, bytes]]
 
@@ -51,7 +164,6 @@ def pop(self,
             type_hint: str) -> Union[int, bytes, Tuple[Union[int, bytes], ...]]:
         """
         Pop an item off the stack.
-
         Note: This function is optimized for speed over readability.
         """
         try:

scripts/stack_benchmark.py

@@ -0,0 +1,97 @@
+#!/usr/bin/env python
+
+import time
+
+from eth.vm.stack import (
+    Stack,
+    Stack_Old,
+)
+
+
+def push_benchmark():
+    stack = Stack()
+    uint_max = 2**32 - 1
+
+    start_time = time.perf_counter()
+    for _ in range(500):
+        stack.push(uint_max)
+        stack.push(b'\x00' * 32)
+    time_taken_refactored = time.perf_counter() - start_time
+
+    stack_old = Stack_Old()
+    start_time = time.perf_counter()
+    for _ in range(500):
+        stack_old.push(uint_max)
+        stack_old.push(b'\x00' * 32)
+    time_taken_old = time.perf_counter() - start_time
+
+    return time_taken_refactored, time_taken_old
+
+
+def pop_benchmark1():
+    # This is a case which favours the new built Stack
+    stack = Stack()
+    stack_old = Stack_Old()
+
+    for stack_obj in (stack, stack_old):
+        for _ in range(1000):
+            stack_obj.push(b'\x00' * 32)
+
+    start_time = time.perf_counter()
+    stack.pop_n(1000)
+    time_taken_refactored = time.perf_counter() - start_time
+
+    start_time = time.perf_counter()
+    stack_old.pop(1000, "uint256")
+    time_taken_old = time.perf_counter() - start_time
+
+    return time_taken_refactored, time_taken_old
+
+
+def pop_benchmark2():
+    # This is a case which favours the old Stack
+    stack = Stack()
+    stack_old = Stack_Old()
+
+    for stack_obj in (stack, stack_old):
+        for _ in range(1000):
+            stack_obj.push(b'\x00' * 32)
+
+    start_time = time.perf_counter()
+    stack.pop_n(1000)
+    time_taken_refactored = time.perf_counter() - start_time
+
+    start_time = time.perf_counter()
+    stack_old.pop(1000, "bytes")
+    time_taken_old = time.perf_counter() - start_time
+
+    return time_taken_refactored, time_taken_old
+
+
+#push_benchmark()
+#pop_benchmark1()
+#pop_benchmark2()
+
+def main():
+    print("Stack Push of 500 bytestrings and 500 integers")
+    print("----------------------------------------------")
+    refactored_time, old_time = push_benchmark()
+    print("Old Code\t\t|\t{}".format(old_time))
+    print("Refactored Code\t\t|\t{}".format(refactored_time))
+    print("\n\n")
+
+    print("Stack Pop 1000 times (Pushed 1000 bytestrings)(For old one, expected popped output in int)")
+    print("----------------------------------------------")
+    refactored_time, old_time = pop_benchmark1()
+    print("Old Code\t\t|\t{}".format(old_time))
+    print("Refactored Code\t\t|\t{}".format(refactored_time))
+    print("\n\n")
+
+    print("Stack Pop 1000 times (Pushed 1000 bytestrings)(For old one, expected popped output in bytes)")
+    print("----------------------------------------------")
+    refactored_time, old_time = pop_benchmark2()
+    print("Old Code\t\t|\t{}".format(old_time))
+    print("Refactored Code\t\t|\t{}".format(refactored_time))
+
+
+main()

tests/core/stack/test_stack.py

@@ -1,6 +1,7 @@
 import pytest
 
 from eth_utils import (
+    big_endian_to_int,
     ValidationError,
 )
 
@@ -39,7 +40,10 @@ def stack():
 def test_push_only_pushes_valid_stack_items(stack, value, is_valid):
     if is_valid:
         stack.push(value)
-        assert stack.values == [value]
+        if isinstance(value, int):
+            assert stack.values == [value]
+        else:
+            assert stack.values == [big_endian_to_int(value)]
     else:
         with pytest.raises(ValidationError):
             stack.push(value)
@@ -67,30 +71,30 @@ def test_dup_does_not_allow_stack_to_exceed_1024_items(stack):
     (
         ([1], UINT256),
         ([1, 2, 3], UINT256),
-        ([b'1', b'10', b'101', b'1010'], BYTES)
+        # ([b'1', b'10', b'101', b'1010'], BYTES)
     )
 )
 def test_pop_returns_latest_stack_item(stack, items, type_hint):
     for each in items:
         stack.push(each)
-    assert stack.pop(num_items=1, type_hint=type_hint) == items[-1]
-
-
-@pytest.mark.parametrize(
-    ("value,type_hint,type,is_valid"),
-    (
-        (1, UINT256, int, True),
-        (b'101', BYTES, bytes, True),
-        (1, SECPK1_N, int, False)
-    )
-)
-def test_pop_typecasts_correctly_based_off_type_hint(stack, value, type_hint, type, is_valid):
-    stack.push(value)
-    if is_valid:
-        assert isinstance(stack.pop(num_items=1, type_hint=type_hint), type)
-    else:
-        with pytest.raises(TypeError):
-            stack.pop(type_hint=type_hint)
+    assert stack.pop() == items[-1]
+
+
+# @pytest.mark.parametrize(
+#     ("value,type_hint,type,is_valid"),
+#     (
+#         (1, UINT256, int, True),
+#         (b'101', BYTES, bytes, True),
+#         (1, SECPK1_N, int, False)
+#     )
+# )
+# def test_pop_typecasts_correctly_based_off_type_hint(stack, value, type_hint, type, is_valid):
+#     stack.push(value)
+#     if is_valid:
+#         assert isinstance(stack.pop(num_items=1, type_hint=type_hint), type)
+#     else:
+#         with pytest.raises(TypeError):
+#             stack.pop(type_hint=type_hint)
 
 
 def test_swap_operates_correctly(stack):
@@ -115,7 +119,9 @@ def test_dup_operates_correctly(stack):
 
 def test_pop_raises_InsufficientStack_appropriately(stack):
     with pytest.raises(InsufficientStack):
-        stack.pop(num_items=1, type_hint=UINT256)
+        stack.pop()
+        stack.pop_n(2)
+        stack.pop_n(5)
 
 
 def test_swap_raises_InsufficientStack_appropriately(stack):