Skip to content

Commit

Permalink
Improve performance of many operations, mainly the Copy/Move file acr…
Browse files Browse the repository at this point in the history 
…oss 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)
  • Loading branch information
@SRombauts
SRombauts committed Mar 8, 2023
1 parent bfad9de commit b902ae5
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
35 changes: 17 additions & 18 deletions Source/PlasticSourceControl/Private/PlasticSourceControlOperations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ bool FPlasticConnectWorker::Execute(FPlasticSourceControlCommand& InCommand)
{
TArray<FString> ContentDir;
ContentDir.Add(FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()));
PlasticSourceControlUtils::RunUpdateStatus(ContentDir, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(ContentDir, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}
}
else
Expand Down Expand Up @@ -249,7 +249,7 @@ bool FPlasticCheckOutWorker::Execute(FPlasticSourceControlCommand& InCommand)
}

// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);

return InCommand.bCommandSuccessful;
}
Expand Down Expand Up @@ -425,7 +425,7 @@ bool FPlasticCheckInWorker::Execute(FPlasticSourceControlCommand& InCommand)
}

// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}
else
{
Expand Down Expand Up @@ -522,7 +522,7 @@ bool FPlasticMarkForAddWorker::Execute(FPlasticSourceControlCommand& InCommand)
}

// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}
else
{
Expand Down Expand Up @@ -566,7 +566,7 @@ bool FPlasticDeleteWorker::Execute(FPlasticSourceControlCommand& InCommand)
}

// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);

return InCommand.bCommandSuccessful;
}
Expand Down Expand Up @@ -694,8 +694,8 @@ bool FPlasticRevertWorker::Execute(FPlasticSourceControlCommand& InCommand)

// NOTE: optim, in UE4 there was no need to update the status of our files since this is done immediately after by the Editor, except now that we are using changelists
#if ENGINE_MAJOR_VERSION == 5
// update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
// update the status of our files. Note: here we need to check for local changes in case of a SoftRevert
PlasticSourceControlUtils::RunUpdateStatus(Files, !bIsSoftRevert, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
#endif

return InCommand.bCommandSuccessful;
Expand Down Expand Up @@ -748,7 +748,7 @@ bool FPlasticRevertUnchangedWorker::Execute(FPlasticSourceControlCommand& InComm
{
Files.Add(FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()));
}
PlasticSourceControlUtils::RunUpdateStatus(Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);

return InCommand.bCommandSuccessful;
}
Expand Down Expand Up @@ -796,8 +796,7 @@ bool FPlasticRevertAllWorker::Execute(FPlasticSourceControlCommand& InCommand)
TArray<FPlasticSourceControlState> TempStates;
TArray<FString> ContentDir;
ContentDir.Add(FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()));
// TODO: here we would just need a fast "status" to only retrieve the local status of files, not server locks etc.
PlasticSourceControlUtils::RunUpdateStatus(ContentDir, false, InCommand.ErrorMessages, TempStates, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(ContentDir, true, false, InCommand.ErrorMessages, TempStates, InCommand.ChangesetNumber, InCommand.BranchName);

for (auto& State : TempStates)
{
Expand Down Expand Up @@ -847,7 +846,7 @@ bool FPlasticRevertAllWorker::Execute(FPlasticSourceControlCommand& InCommand)
// now update the status of the updated files
if (Operation->UpdatedFiles.Num())
{
PlasticSourceControlUtils::RunUpdateStatus(Operation->UpdatedFiles, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(Operation->UpdatedFiles, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}

return InCommand.bCommandSuccessful;
Expand Down Expand Up @@ -934,7 +933,7 @@ bool FPlasticUpdateStatusWorker::Execute(FPlasticSourceControlCommand& InCommand

if (Files.Num() > 0)
{
InCommand.bCommandSuccessful = PlasticSourceControlUtils::RunUpdateStatus(Files, Operation->ShouldUpdateHistory(), InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
InCommand.bCommandSuccessful = PlasticSourceControlUtils::RunUpdateStatus(Files, false, Operation->ShouldUpdateHistory(), InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
// Remove all "is not in a workspace" error and convert the result to "success" if there are no other errors
PlasticSourceControlUtils::RemoveRedundantErrors(InCommand, TEXT("is not in a workspace."));
if (!InCommand.bCommandSuccessful)
Expand Down Expand Up @@ -1000,7 +999,7 @@ bool FPlasticUpdateStatusWorker::Execute(FPlasticSourceControlCommand& InCommand
{
TArray<FString> ProjectDirs;
ProjectDirs.Add(FPaths::ConvertRelativePathToFull(FPaths::ProjectContentDir()));
InCommand.bCommandSuccessful = PlasticSourceControlUtils::RunUpdateStatus(ProjectDirs, Operation->ShouldUpdateHistory(), InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
InCommand.bCommandSuccessful = PlasticSourceControlUtils::RunUpdateStatus(ProjectDirs, false, Operation->ShouldUpdateHistory(), InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}
else
{
Expand Down Expand Up @@ -1162,11 +1161,11 @@ bool FPlasticCopyWorker::Execute(FPlasticSourceControlCommand& InCommand)
InCommand.bCommandSuccessful = true;
}

// now update the status of our files:
// now update the status of our files
TArray<FString> BothFiles;
BothFiles.Add(Origin);
BothFiles.Add(Destination);
PlasticSourceControlUtils::RunUpdateStatus(BothFiles, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(BothFiles, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}
else
{
Expand Down Expand Up @@ -1198,7 +1197,7 @@ bool FPlasticSyncWorker::Execute(FPlasticSourceControlCommand& InCommand)
// now update the status of the updated files
if (UpdatedFiles.Num())
{
PlasticSourceControlUtils::RunUpdateStatus(UpdatedFiles, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(UpdatedFiles, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
}

if ((InCommand.Operation->GetName() == FName("SyncAll")))
Expand Down Expand Up @@ -1252,7 +1251,7 @@ bool FPlasticResolveWorker::Execute(FPlasticSourceControlCommand& InCommand)
}

// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);

return InCommand.bCommandSuccessful;
}
Expand Down Expand Up @@ -2018,7 +2017,7 @@ bool FPlasticUnshelveWorker::Execute(FPlasticSourceControlCommand& InCommand)
if (InCommand.bCommandSuccessful)
{
// now update the status of our files
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);
PlasticSourceControlUtils::RunUpdateStatus(InCommand.Files, true, false, InCommand.ErrorMessages, States, InCommand.ChangesetNumber, InCommand.BranchName);

ChangelistToUpdate = InCommand.Changelist;
}
Expand Down
19 changes: 14 additions & 5 deletions Source/PlasticSourceControl/Private/PlasticSourceControlUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,12 +594,13 @@ class FFileVisitor : public IPlatformFile::FDirectoryVisitor
*
* @param[in] InDir The path to the common directory of all the files listed after.
* @param[in] InFiles List of files in a directory, or the path to the directory itself (never empty).
* @param[in] bInControlledOnly Call "status" with "--controlledchanged" instead of "--all" when doing only a quick check following a source control operation
* @param[out] OutErrorMessages Error messages from the "status" command
* @param[out] OutStates States of files for witch the status has been gathered (distinct than InFiles in case of a "directory status")
* @param[out] OutChangeset The current Changeset Number
* @param[out] OutBranchName Name of the current checked-out branch
*/
static bool RunStatus(const FString& InDir, TArray<FString>&& InFiles, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName)
static bool RunStatus(const FString& InDir, TArray<FString>&& InFiles, const bool bInControlledOnly, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName)
{
TRACE_CPUPROFILER_EVENT_SCOPE(PlasticSourceControlUtils::RunStatus);

Expand All @@ -608,8 +609,16 @@ static bool RunStatus(const FString& InDir, TArray<FString>&& InFiles, TArray<FS
TArray<FString> Parameters;
Parameters.Add(TEXT("--compact"));
Parameters.Add(TEXT("--noheaders"));
Parameters.Add(TEXT("--all"));
Parameters.Add(TEXT("--ignored"));
if (bInControlledOnly)
{
Parameters.Add(TEXT("--controlledchanged"));
}
else
{
Parameters.Add(TEXT("--all"));
Parameters.Add(TEXT("--ignored"));
}

// "cm status" only operate on one path (file or directory) at a time, so use one common path for multiple files in a directory
TArray<FString> OnePath;
// Only one file: optim very useful for the .uproject file at the root to avoid parsing the whole repository
Expand Down Expand Up @@ -952,7 +961,7 @@ struct FFilesInCommonDir
};

// Run a batch of Plastic "status" and "fileinfo" commands to update status of given files and directories.
bool RunUpdateStatus(const TArray<FString>& InFiles, const bool bInUpdateHistory, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName)
bool RunUpdateStatus(const TArray<FString>& InFiles, const bool bInControlledOnly, const bool bInUpdateHistory, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName)
{
bool bResults = true;

Expand Down Expand Up @@ -1047,7 +1056,7 @@ bool RunUpdateStatus(const TArray<FString>& InFiles, const bool bInUpdateHistory
// Run a "status" command on the directory to get workspace file states.
// (ie. Changed, CheckedOut, Copied, Replaced, Added, Private, Ignored, Deleted, LocallyDeleted, Moved, LocallyMoved)
TArray<FPlasticSourceControlState> States;
const bool bGroupOk = RunStatus(Group.Value.CommonDir, MoveTemp(Group.Value.Files), OutErrorMessages, States, OutChangeset, OutBranchName);
const bool bGroupOk = RunStatus(Group.Value.CommonDir, MoveTemp(Group.Value.Files), bInControlledOnly, OutErrorMessages, States, OutChangeset, OutBranchName);
if (!bGroupOk)
{
bResults = false;
Expand Down
3 changes: 2 additions & 1 deletion Source/PlasticSourceControl/Private/PlasticSourceControlUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,15 @@ bool RunCommand(const FString& InCommand, const TArray<FString>& InParameters, c
* Run a Plastic "status" command and parse it.
*
* @param InFiles The files to be operated on
* @param bInControlledOnly Call "status" with "--controlledchanged" instead of "--all" when doing only a quick check following a source control operation
* @param bInUpdateHistory If getting the history of files, force execute the fileinfo command required to do get RepSpec of xlinks (history view or visual diff)
* @param OutErrorMessages Any errors (from StdErr) as an array per-line
* @param OutStates States of the files
* @param OutChangeset The current Changeset Number
* @param OutBranchName Name of the current checked-out branch
* @returns true if the command succeeded and returned no errors
*/
bool RunUpdateStatus(const TArray<FString>& InFiles, const bool bInUpdateHistory, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName);
bool RunUpdateStatus(const TArray<FString>& InFiles, const bool bInControlledOnly, const bool bInUpdateHistory, TArray<FString>& OutErrorMessages, TArray<FPlasticSourceControlState>& OutStates, int32& OutChangeset, FString& OutBranchName);

/**
* Run a Plastic "cat" command to dump the binary content of a revision into a file.
Expand Down

0 comments on commit b902ae5

Please sign in to comment.