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

Remove some of the @ from the AST #7615

Merged
merged 10 commits into from
Jul 7, 2013
Merged

Conversation

Aatch
Copy link
Contributor

@Aatch Aatch commented Jul 6, 2013

In an ideal world, the AST would be completely sendable, this gets us a step closer.

It removes the local heap allocations for view_item, Path, Lifetime trait_ref OptVec<TyParamBounds> and Ty. There are also a few other smaller changes I made as things went along.

@emberian
Copy link
Member

emberian commented Jul 6, 2013

😍

@Aatch
Copy link
Contributor Author

Aatch commented Jul 6, 2013

This won't pass the tests as-is, since it unsuprisingly broke rustdoc (and it'll probably fail some other tests that I haven't run yet) I just wanted to get it up for feedback.

@graydon
Copy link
Contributor

graydon commented Jul 6, 2013

Wonderful. r=me with p=100 on anything that has this form, assuming it doesn't introduce lots of new deep-copy-making. Especially keen on seeing the session object un-@'ed, since it's a bottleneck to splitting phases into subtasks.

@emberian
Copy link
Member

emberian commented Jul 6, 2013

benchmark

@Aatch
Copy link
Contributor Author

Aatch commented Jul 6, 2013

@cmr, not the change I was hoping for, but oh well. The extra time at the end is odd, but I'm going to cop out and blame noise.

Aatch and others added 10 commits July 7, 2013 22:51
Also, makes the pretty-printer use & instead of @ as much as possible,
which will help with later changes, though in the interim has produced
some... interesting constructs.
bors added a commit that referenced this pull request Jul 7, 2013
In an ideal world, the AST would be completely sendable, this gets us a step closer.

It removes the local heap allocations for `view_item`, `Path`, `Lifetime` `trait_ref` `OptVec<TyParamBounds>` and `Ty`. There are also a few other smaller changes I made as things went along.
@bors bors closed this Jul 7, 2013
@bors bors merged commit 280e424 into rust-lang:master Jul 7, 2013
@huonw
Copy link
Member

huonw commented Jul 7, 2013

Apparently, this adds 300 MB to the memory usage, but I guess it's a case of gets-worse-before-it-gets-better.

@Aatch Aatch mentioned this pull request Jul 8, 2013
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.

5 participants