Skip to content

Remove uneccessary AST copy - #1357

Merged
levkk merged 2 commits into
mainfrom
sg-remove-copy
Aug 13, 2026
Merged

Remove uneccessary AST copy#1357
levkk merged 2 commits into
mainfrom
sg-remove-copy

Conversation

@sgrif

@sgrif sgrif commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

We were previously parsing an AST only to immediatley copy it onto a new memory arena and throw the original away. I've added a function on the parser side to allow us to skip this step. We can start using it now

We were previously parsing an AST only to immediatley copy it onto a new
memory arena and throw the original away. I've added a function on the
parser side to allow us to skip this step. We can start using it now
@sgrif
sgrif requested a review from levkk August 13, 2026 18:10

@levkk levkk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh neat! This might even register on a benchmark

@sgrif

sgrif commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

It's probably pretty minor, but yeah cutting ~50% of the allocations should be a visible change

@levkk

levkk commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

The trick is testing it with a big query:

image

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@levkk

levkk commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

I'm going to merge this optimization to include it in today's release. Let me know if there is anything else needed here.

@levkk
levkk merged commit db11bfd into main Aug 13, 2026
28 checks passed
@levkk
levkk deleted the sg-remove-copy branch August 13, 2026 19:35
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