Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce contract code size by wrapping clamping of args into a loop #1488

Merged
merged 3 commits into from Jul 18, 2019

Conversation

@siraben
Copy link
Contributor

siraben commented Jun 20, 2019

What I did

Reduced contract code size due to codegen of run-time type checking of list elements.

How I did it

Changed make_arg_clamper to generate an LLL loop instead of recursing over the list elements, when there are more than 5 elements in the list.

All the tests pass.

How to verify it

Compile the following contract.

struct Animal:
    Name: uint256
    Exists: int128

COLLECTION_SIZE: constant(uint256) = 1000

contractOwner: address
daddy: address
collection: map(address, Animal)

count: int128

@private
@constant
def isZookeeper(sender: address) -> bool:
    return sender == self.contractOwner or sender == self.daddy

@public
def addToCollection(animals: address[COLLECTION_SIZE]):
    assert self.isZookeeper(msg.sender)

    for animal in animals:
        if animal == ZERO_ADDRESS: break

        if self.collection[animal].Exists == 0:
            self.count += 1
            self.collection[animal].Exists = self.count

@public
def __init__(myDaddy: address):
    self.daddy = myDaddy

Old code size (bytes): 55340
New code size (bytes): 17474
This PR combined with #1486 reduces the code size down to a whopping 1644 bytes

Cute Animal Picture

It's a very nice! I like!

Co-Authored-By: Charles Cooper <cooper.charles.m@gmail.com>
for i in range(typ.count):
offset = get_size_of_type(typ.subtype) * 32 * i
o.append(make_arg_clamper(datapos + offset, mempos + offset, typ.subtype, is_init))
if typ.count > 5 or (type(datapos) is list and type(mempos) is list):

This comment has been minimized.

Copy link
@charles-cooper

charles-cooper Jun 22, 2019

Collaborator

when are datapos and mempos lists?

This comment has been minimized.

Copy link
@siraben

siraben Jun 22, 2019

Author Contributor

When we have a list of lists! This was revealed to me in one of the test cases.

This comment has been minimized.

Copy link
@charles-cooper

charles-cooper Jun 22, 2019

Collaborator

which test case?

This comment has been minimized.

Copy link
@jacqueswww

jacqueswww Jul 4, 2019

Collaborator

@siraben I think you should just use get_size_of_type(typ) here instead this will support above cases as well.

Copy link
Collaborator

jacqueswww left a comment

LGTM. Just one change in the if statement.

for i in range(typ.count):
offset = get_size_of_type(typ.subtype) * 32 * i
o.append(make_arg_clamper(datapos + offset, mempos + offset, typ.subtype, is_init))
if typ.count > 5 or (type(datapos) is list and type(mempos) is list):

This comment has been minimized.

Copy link
@jacqueswww

jacqueswww Jul 4, 2019

Collaborator

@siraben I think you should just use get_size_of_type(typ) here instead this will support above cases as well.

@jacqueswww

This comment has been minimized.

Copy link
Collaborator

jacqueswww commented Jul 18, 2019

Merging now, will add get_size_of_type separately.

@jacqueswww jacqueswww merged commit 4152d1d into ethereum:master Jul 18, 2019
3 checks passed
3 checks passed
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.