Skip to content

Conversation

@samsonasik
Copy link
Member

It seems the leaveNode seems no longer needed on both AbstractRector and CallableNodeVisitor, also its save nodes to returns and node id to remove properties.

This PR to remove it to both classes.

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Member

I've always wanted to remove this mess. Thank you 🙏

Could you test on some old version of a project, where dead-code set removes lot of code? I wonder what performance is before/after.

Some code has to be removed by the dead-code set, so we can see this part being used.

@samsonasik
Copy link
Member Author

I tested in our project with 5617 files with 101 files changed:

Before: 1 minutes 52 seconds

➜  ../rector-src/bin/rector process --clear-cache
                                                                                                                        
 [OK] 101 files have been changed by Rector                                                                             
                                                                                                                        
../rector-src/bin/rector process --clear-cache  1043.92s user 32.09s system 953% cpu 1:52.80 total

After: 1 minutes 50 seconds

➜  ../rector-src/bin/rector process --clear-cache
                                                                                                                        
 [OK] 101 files have been changed by Rector                                                                            
                                                                                                                        
../rector-src/bin/rector process --clear-cache  1037.09s user 32.84s system 968% cpu 1:50.45 total

The different seems not really noticable as only 2 seconds, it just clean up if possible.

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