Skip to content

Conversation

@Earlopain
Copy link
Collaborator

Allows to compare the output of two different prism versions.

It compiles prism twice and forks to allow for different versions in the same script. It then passes over minimal data to see if anything has changed.

Most bugfixes should impact little to no real code and the test suite is already very extensive. Running this can give you even more confidence by comparing against real-world-rails or similar.

There are some performance gains to be had here. Basically it is already parallelized because of fork but it can be even better. For simplicity (and because I don't usually write such code) I leave that as an exercise for the future. The message on change can also be improved, but right now I'm not certain that's worth the effort. Just check it manually via the already existing tools.

Would have shown that #3669 isn't quite right.

@Earlopain Earlopain force-pushed the test-real-world branch 3 times, most recently from 9ff28ed to d9c2633 Compare November 29, 2025 16:35
@Earlopain
Copy link
Collaborator Author

I added a custom visitor so that this runs faster. It gets me from ~570 down to ~98 seconds for real-world-rails (~138k source files).

I looked to optimize the inspect visitor but doubt it would get close to a 6x improvement. That said, maybe there is some other approach that I missed? It just needs to produce the same output given the same ast and doesn't need to care much about collisions since it's more like a parity check. I tried using digest to avoid appending to the string each time but that turned out to be slower.

Copy link
Collaborator

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

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

Should we compare the serialized string instead of the inspect output? That might simplify things.

Allows to compare the output of two different prism versions.

It compiles prism twice and forks to allow for different versions in the same script.
It then passes over minimal data to see if anything has changed.

Most bugfixes should impact little to no real code and the test suite is already very extensive.
Running this can give you even more confidence by comparing against real-world-rails or similar.

There are some performance gains to be had here. Basically it is already parallelized because
of `fork` but it can likely be even better. For simplicity (and because I don't usually
write such code) I leave that as an exercise for the future.
Just check it manually via the already existing tools.
@Earlopain
Copy link
Collaborator Author

Damn, wish I thought of that. It's not 100% equivalent (x("foo", "bar") and x("bar", "foo") for example produce the same dump output) but for the purpose of this check that doesn't really matter in my eyes. I'm more interested in if nodes trade places, which this still can do no problem.

It now checks real-world-rails in just 12 seconds for me (ignoring compilation) which is pretty impressive.

@kddnewton kddnewton merged commit eb4a823 into ruby:main Dec 1, 2025
64 checks passed
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