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

Simplify spreads of inline arrays #467

Merged
merged 1 commit into from
Aug 26, 2019

Conversation

Schaeff
Copy link
Member

@Schaeff Schaeff commented Aug 26, 2019

There is a memory blowup in the way we desugar spreads:

size(...a) = n * size(a)

because

...a = a[0], ..., a[n] <-- we have to copy a n times when we desugar

but if a is an inline value like [0, 1, 2]

size(a) = O(n)

With this change, we desugar ...a when a is an inline array to the content of the array:

...[x, y, z] = x, y, z

The program

def main() -> ():
	field[3] a = [...[...[...[0, 0, 0]]]]
	return

Before this change:

def main() -> ():
	field[3] _a_0
	_a_0 = [[[[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][0], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][1], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][2]][0], [[[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][0], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][1], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][2]][1], [[[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][0], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][1], [[0, 0, 0][0], [0, 0, 0][1], [0, 0, 0][2]][2]][2]]
	return 

After this change:

def main() -> ():
	field[3] _a_0
	_a_0 = [0, 0, 0]
	return 

Replaces #462 which ends up being too complex of an approach.

Copy link
Member

@JacobEberhardt JacobEberhardt left a comment

Choose a reason for hiding this comment

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

Low-hanging fruit, nice!

@JacobEberhardt JacobEberhardt merged commit 70d38d7 into develop Aug 26, 2019
@JacobEberhardt JacobEberhardt deleted the simplify-inline-array-spreads branch August 26, 2019 11:41
@Schaeff Schaeff mentioned this pull request Oct 8, 2019
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.

2 participants