Skip to content

Conversation

FractalFir
Copy link
Contributor

This PR changes a few things.

First of all, it caches ConstAllocations, ensuring that for a given ConstAllocation(or one an identical one with a different aligement), you will get the exact same Rvalue.

This allows us to simplify the deduplication code in static_addr_of, and remove the formating (which caused memory leaks).

Additonally, this allows constant byte sequences to sometimes be represented in a more compact fashion. From a few checks, it looks like ~80 % of constant byte sequences can be optimized this way.

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Very nice work!

Here's a first review for the ConstAllocation part.

@FractalFir
Copy link
Contributor Author

FractalFir commented May 21, 2025

As a nice bonus, this PR seems to drastically reduce the ammount of memory used to build regex-automata.

I can now build rustc with 10 threads, and run on ~28 GB of RAM. This is still suboptimal, but a fair bit better than before.

@antoyo
Copy link
Contributor

antoyo commented May 21, 2025

As a nice bonus, this PR seems to drastically reduce the ammount of memory used to build regex-automata.

I can now build rustc with 10 threads, and run on ~28 GB of RAM. This is still suboptimal, but a fair bit better than before.

Very nice improvement!
Is this with -Copt-level=0 and do you still disable some GCC passes?

@FractalFir
Copy link
Contributor Author

As a nice bonus, this PR seems to drastically reduce the ammount of memory used to build regex-automata.
I can now build rustc with 10 threads, and run on ~28 GB of RAM. This is still suboptimal, but a fair bit better than before.

Very nice improvement! Is this with -Copt-level=0 and do you still disable some GCC passes?

On -Copt-level=1

@FractalFir
Copy link
Contributor Author

Should be good to merge now.

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

After this, it should be good to merge.
Thanks!

Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

A few nitpicks:

@FractalFir FractalFir force-pushed the master branch 2 times, most recently from 1616001 to 4efa778 Compare May 21, 2025 17:26
Copy link
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

After this last nitpick is fixed, I believe I can merge.
Thanks!

@bjorn3
Copy link
Member

bjorn3 commented May 21, 2025

In your commit message: *representation

@FractalFir
Copy link
Contributor Author

In your commit message: *representation

Did not see that, thanks :)!

@antoyo antoyo merged commit 3d962df into rust-lang:master May 21, 2025
37 checks passed
@antoyo
Copy link
Contributor

antoyo commented May 21, 2025

Thanks a lot for this nice improvement!

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.

3 participants