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

seq: optimize append #280

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sorawee
Copy link
Collaborator

@sorawee sorawee commented Mar 12, 2024

There are two changes in this commit.

The first change builds the list from a variadic append operation from right to left, which is the natural order for list construction. This speeds up the following program:

(time
 (void
  (append* (for/list ([i 30000])
             (if b '() '(1))))))

from 5 seconds to 1 second.

The second change avoids making an assertion when doing unsafe/append on two symbolic unions. This is fine because all calls to unsafe/append is already guarded via type casting to make them symbolic unions of rosette-contract?, so there is no need to add another guard.

Thanks to @camoy for raising up the issue about the unnecessary assertions.

There are two changes in this commit.

The first change builds the list from a variadic append operation
from right to left, which is the natural order for list construction.
This speeds up the following program:

```
(time
 (void
  (append* (for/list ([i 30000])
             (if b '() '(1))))))
```

from 5 seconds to 1 second.

The second change avoids making an assertion when doing unsafe/append
on two symbolic unions. This is fine because all calls to unsafe/append
is already guarded via type casting to make them symbolic unions
of rosette-contract?, so there is no need to add another guard.

Thanks to @camoy for raising up the issue about the unnecessary assertions.
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.

1 participant