Skip to content

Reduce size of InferenceResult #19891

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Reduce size of InferenceResult #19891

wants to merge 5 commits into from

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented May 29, 2025 •
edited
Loading

No description provided.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2025
@ChayimFriedman2
Copy link
Contributor

It's probably better to keep this as a single struct, especially given that there are other eligible types, e.g. usize for array lengths.

@Veykril
Copy link
Member Author

Veykril commented May 29, 2025

Right, good call. Would be good to check how this would look like with the new type ir as well I think

@Veykril Veykril force-pushed the push-krwmpzoropoz branch from 60d3580 to 1a5f08f Compare May 29, 2025 14:15
@Veykril
Copy link
Member Author

Veykril commented May 29, 2025

Don't mind the other commits for now, I am experimenting with how far we can shrink InferenceResult (568 bytes). I have gotten it down to 368 so far.

@Veykril Veykril changed the title Remove InternedStandardTypes Reduce size of InferenceResult May 29, 2025
@ChayimFriedman2
Copy link
Contributor

So I was curious and checked the impact on memory. This saves 4 megabytes 😞 I'm sorry to say I don't think it's worth the effort.

@Veykril
Copy link
Member Author

Veykril commented May 30, 2025

Not surprising, we do not have that many InferenceResults

@Veykril
Copy link
Member Author

Veykril commented May 30, 2025

I do think most of the changes here are still good either way but let's put this to the side until the trait solver lands

@Veykril Veykril marked this pull request as draft May 30, 2025 05:57
@ChayimFriedman2
Copy link
Contributor

Not surprising, we do not have that many InferenceResults

We don't? In analysis-stats every function has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Veykril @ChayimFriedman2 @rustbot