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

Streamline append VM implementation #1646

Merged
merged 8 commits into from
Nov 2, 2024
Merged

Conversation

wycats
Copy link
Contributor

@wycats wycats commented Oct 31, 2024

Attempting to pull out the commit that doesn't seem to have a standalone performance impact so we can land it separately.

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

Oh no, regression!

@NullVoxPopuli
Copy link
Contributor

I ran it twice

image

image

@wycats wycats added the perf label Nov 1, 2024
The fact that the build was conflating two unrelated Stack types is a
bug, and we'll fix it by using the standard rollup plugin soon.

For now, we're going to work around it by renaming the internal Stack
type in VM to VmStack.
@NullVoxPopuli NullVoxPopuli added perf and removed perf labels Nov 2, 2024
@NullVoxPopuli
Copy link
Contributor

We both tested this locally, and perf is fine

@wycats wycats merged commit 0d5ec41 into main Nov 2, 2024
7 of 10 checks passed
@wycats wycats deleted the infra/streamline-append-vm branch November 2, 2024 23:49
@github-actions github-actions bot mentioned this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants