From 2358e8b5c56277126a36a634d5adc88aee26882d Mon Sep 17 00:00:00 2001 From: Bhargavasomu Date: Fri, 21 Dec 2018 12:54:16 +0530 Subject: [PATCH 1/3] Preliminary check for test breaks --- eth/vm/computation.py | 40 ++++++++++++- eth/vm/stack.py | 102 ++++++++++++++++++++++----------- tests/core/stack/test_stack.py | 48 +++++++++------- 3 files changed, 134 insertions(+), 56 deletions(-) diff --git a/eth/vm/computation.py b/eth/vm/computation.py index a6ca25a3da..20037dabcf 100644 --- a/eth/vm/computation.py +++ b/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: """ diff --git a/eth/vm/stack.py b/eth/vm/stack.py index 2c9028b205..711efb7e44 100644 --- a/eth/vm/stack.py +++ b/eth/vm/stack.py @@ -30,7 +30,7 @@ class Stack(object): logger = logging.getLogger('eth.vm.stack.Stack') def __init__(self) -> None: - self.values = [] # type: List[Union[int, bytes]] + self.values = [] # type: List[int] def __len__(self) -> int: return len(self.values) @@ -44,47 +44,81 @@ def push(self, value: Union[int, bytes]) -> None: validate_stack_item(value) - self.values.append(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)) + ) - def pop(self, - num_items: int, - type_hint: str) -> Union[int, bytes, Tuple[Union[int, bytes], ...]]: - """ - Pop an item off the stack. + self.values.append(stack_value) - Note: This function is optimized for speed over readability. - """ + # 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: - if num_items == 1: - return next(self._pop(num_items, type_hint)) - else: - return tuple(self._pop(num_items, type_hint)) + return self.values.pop() except IndexError: raise InsufficientStack("No stack items") - def _pop(self, num_items: int, type_hint: str) -> Generator[Union[int, bytes], None, None]: + def pop_n(self, num_items: int) -> Tuple[int, ...]: 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: + try: yield self.values.pop() - else: - raise TypeError( - "Unknown type_hint: {0}. Must be one of {1}".format( - type_hint, - ", ".join((constants.UINT256, constants.BYTES)), - ) - ) + except IndexError: + raise InsufficientStack("No stack items") + + def pop2(self) -> Tuple[int, int]: + try: + return (self.values.pop(), self.values.pop()) + except IndexError: + raise InsufficientStack("No stack items") + + def pop3(self) -> Tuple[int, int, int]: + try: + return (self.values.pop(), self.values.pop(), self.values.pop()) + except IndexError: + raise InsufficientStack("No stack items") def swap(self, position: int) -> None: """ diff --git a/tests/core/stack/test_stack.py b/tests/core/stack/test_stack.py index 1e81839530..592ab413dc 100644 --- a/tests/core/stack/test_stack.py +++ b/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): From c5c786332e0ee63b00662043f6a2c9c0bae78c9e Mon Sep 17 00:00:00 2001 From: Bhargavasomu Date: Fri, 21 Dec 2018 18:03:55 +0530 Subject: [PATCH 2/3] Add benchmarks for showing time --- .circleci/config.yml | 7 +++ eth/vm/stack.py | 88 +++++++++++++++++++++++++++++++++-- scripts/stack_benchmark.py | 95 ++++++++++++++++++++++++++++++++++++++ tox.ini | 8 +++- 4 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 scripts/stack_benchmark.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 7e19e8ee9f..0112904d85 100644 --- a/.circleci/config.yml +++ b/.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 diff --git a/eth/vm/stack.py b/eth/vm/stack.py index 711efb7e44..f76e5d4d68 100644 --- a/eth/vm/stack.py +++ b/eth/vm/stack.py @@ -102,24 +102,102 @@ def pop(self) -> int: 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 pop2(self) -> Tuple[int, int]: + def swap(self, position: int) -> None: + """ + Perform a SWAP operation on the stack. + """ + idx = -1 * position - 1 try: - return (self.values.pop(), self.values.pop()) + self.values[-1], self.values[idx] = self.values[idx], self.values[-1] except IndexError: - raise InsufficientStack("No stack items") + raise InsufficientStack("Insufficient stack items for SWAP{0}".format(position)) - def pop3(self) -> Tuple[int, int, int]: + def dup(self, position: int) -> None: + """ + Perform a DUP operation on the stack. + """ + idx = -1 * position try: - return (self.values.pop(), self.values.pop(), self.values.pop()) + 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]] + + 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) + + self.values.append(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 swap(self, position: int) -> None: """ Perform a SWAP operation on the stack. diff --git a/scripts/stack_benchmark.py b/scripts/stack_benchmark.py new file mode 100644 index 0000000000..4f5119037e --- /dev/null +++ b/scripts/stack_benchmark.py @@ -0,0 +1,95 @@ +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() diff --git a/tox.ini b/tox.ini index 5ebbcfc1bf..cf39fd2898 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,7 @@ [tox] envlist= py{35,36}-{core,database,transactions,vm} - py{36}-{benchmark,beacon} + py{36}-{benchmark,beacon,stack-benchmark} py{35,36}-native-blockchain-{frontier,homestead,eip150,eip158,byzantium,constantinople,metropolis,transition} py37-{core,beacon} py{35,36}-lint @@ -57,6 +57,12 @@ commands= benchmark: {toxinidir}/scripts/benchmark/run.py +[testenv:py36-stack-benchmark] +deps = .[eth-extra,stack-benchmark] +commands= + stack-benchmark: {toxinidir}/scripts/stack_benchmark.py + + [common-lint] deps = .[eth,lint] setenv=MYPYPATH={toxinidir}:{toxinidir}/stubs From 16560c9ccdd583dfc6f8c0e7b97f56e10b2cfaac Mon Sep 17 00:00:00 2001 From: Bhargavasomu Date: Thu, 27 Dec 2018 16:06:42 +0530 Subject: [PATCH 3/3] Give executable permissions to stack_benchmark --- scripts/stack_benchmark.py | 2 ++ 1 file changed, 2 insertions(+) mode change 100644 => 100755 scripts/stack_benchmark.py diff --git a/scripts/stack_benchmark.py b/scripts/stack_benchmark.py old mode 100644 new mode 100755 index 4f5119037e..35185d6785 --- a/scripts/stack_benchmark.py +++ b/scripts/stack_benchmark.py @@ -1,3 +1,5 @@ +#!/usr/bin/env python + import time from eth.vm.stack import (