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

Add some folders which would benefit from scu to scu pipeline #98261

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

dustdfg
Copy link
Contributor

@dustdfg dustdfg commented Oct 17, 2024

Used SCU and saw that some big/medium size folders aren't processed with SCU

@dustdfg dustdfg requested a review from a team as a code owner October 17, 2024 10:54
@dustdfg dustdfg changed the title Add some big folders which would benefit from scu to scu pipeline Add some folders which would benefit from scu to scu pipeline Oct 17, 2024
@AThousandShips AThousandShips added this to the 4.x milestone Oct 17, 2024
Signed-off-by: Yevhen Babiichuk (DustDFG) <dfgdust@gmail.com>
@dustdfg dustdfg force-pushed the scu_additional_folders branch from 9dcc8f8 to 8eeee92 Compare October 30, 2024 20:50
@clayjohn clayjohn requested a review from lawnjelly October 30, 2024 21:49
@lawnjelly
Copy link
Member

lawnjelly commented Oct 31, 2024

Yes it's super possible that this will be a good idea, as there have been folder changes / new folders since the original SCU PR, and we may be missing out on speedup. I don't tend to monitor this much as I mainly work on Godot 3.x.

Timings for full rebuilds before / after are particularly useful for this kind of thing.

I'll try the PR. 👍

@dustdfg
Copy link
Contributor Author

dustdfg commented Oct 31, 2024

It is only several folders but they contain many files. Timings of my rebuilds usually take 30min so honestly it is not what I really want to measure (because to make consistent measurements I need 1h doing nothing with device)... I can make a screenshots of the build process where lots of lines are not scu...

@lawnjelly
Copy link
Member

On my machine with clean builds:

Before: 2mins 54
After: 2mins 39

Some of the folders added won't make a whole lot of difference (where there are e.g. just 2 cpp files) but on the whole this seems worth doing. The checks indicate it compiles fine with different platforms but we should watch for any build regressions in the next few days after merging.

Other than that it should be pretty safe if no name conflicts are detected by compiler.

@Repiteo Repiteo modified the milestones: 4.x, 4.4 Nov 1, 2024
Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

@Repiteo Repiteo merged commit 980df1f into godotengine:master Nov 1, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 1, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants