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

Improve performances of "move" operations by calling status with --controlledchanged instead of --all #69

Merged
merged 2 commits into from Mar 14, 2023

Conversation

SRombautsU
Copy link

@SRombautsU SRombautsU commented Mar 8, 2023

Improve performance of many operations, mainly the Copy/Move file across folders

Optimize RunUpdateStatus() with a new parameter bInControlledOnly to call "status" with the flag "--controlledchanged" instead of "--all" when doing a quick check following a source control operation,
as opposed to the specific case of the "Update Status" operation called by the Editor to update the status of files in the content browser.

Rational: When checking the status of files after a source control operation, we mostly only care of controlled status (Controlled, CheckedOut, Added, Moved, Deleted)


Initial attempt / workaround bellow;

Improve performances of "move" by not grouping the final "status" on both source & destination files if they are not on the same folder.

Reasoning: the final "status" call on the common directory of both files could very well operate on the root Content/ folder itself, leading to an expensive call checking status of the whole project

As there is no perfect heuristic to know when a directory status is more expensive that two file status, let's assume that a "rename" (ie. source & destination in the same directory) is always worth grouping, while a "move" (different directories) is more at risk to create a huge status update.

This has the potential to help a lot on big game project, where a call to a "full Content/ status" can take seconds (so moving 100 assets might take 1000s less). The downside is that it will slow down the status on small project where it only takes around 100ms (where moving 100 assets might take 10s more).

@SRombautsU SRombautsU self-assigned this Mar 8, 2023
@SRombautsU SRombautsU force-pushed the 1001028-improve-move-status-performances branch 2 times, most recently from 2d303cd to b902ae5 Compare March 8, 2023 20:29
@SRombautsU SRombautsU changed the title Improve performances of "move" by not grouping the final "status" on both source & destination files if they are not on the same folder. Improve performances of "move" operations by calling status with --controlledchanged instead of --all Mar 8, 2023
@SRombautsU SRombautsU force-pushed the 1001028-improve-move-status-performances branch from b902ae5 to 64f25a7 Compare March 9, 2023 09:58
…oss folders

Optimize RunUpdateStatus() with a new parameter InSearchType to call "status" with the flag "--controlledchanged" instead of "--all" when doing a quick check following a source control operation,
as opposed to the specific case of the "Update Status" operation called by the Editor to update the status of files in the content browser.

Rational: When checking the status of files after a source control operation, we mostly only care of controlled status (Controlled, CheckedOut, Added, Moved, Deleted)
@SRombautsU SRombautsU force-pushed the 1001028-improve-move-status-performances branch from 64f25a7 to e19401c Compare March 9, 2023 10:11
Copy link

@mig42 mig42 left a comment

Choose a reason for hiding this comment

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

LGTM

…e::All in order to list all local changes as well before running the undo operation

Found during the review/validation session
@mig42 mig42 merged commit 15b900f into master Mar 14, 2023
@mig42 mig42 deleted the 1001028-improve-move-status-performances branch March 14, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants