Skip to content

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

Merged
jacqueswww merged 3 commits into
vyperlang:masterfrom
siraben:clamp-args-fix
Jul 18, 2019
Merged

Reduce contract code size by wrapping clamping of args into a loop#1488
jacqueswww merged 3 commits into
vyperlang:masterfrom
siraben:clamp-args-fix

Conversation

@siraben

@siraben siraben commented Jun 20, 2019

Copy link
Copy Markdown
Contributor

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!

Comment thread vyper/parser/arg_clamps.py Outdated
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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when are datapos and mempos lists?

@siraben siraben Jun 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which test case?

@jacqueswww jacqueswww Jul 4, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jacqueswww jacqueswww left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

@jacqueswww jacqueswww Jul 4, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jacqueswww

Copy link
Copy Markdown
Contributor

Merging now, will add get_size_of_type separately.

@jacqueswww jacqueswww merged commit 4152d1d into vyperlang:master Jul 18, 2019
@siraben siraben deleted the clamp-args-fix branch November 25, 2020 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants