#rb Per.Larsson
#jira UE-182205
#preflight 643932b2211b661dc4f594c6
- The enums don't contain any entries at the moment, the goal of this change is to reduce the friction that future API changes might have.
- The older versions of PullData/PushData have been deprecated but this won't flag compiler warnings if people have derived their own implementations of IVirtualizationBackend, so in addition to deprecating the methods have been set to 'final' and have checkNoEntry implementations which is the best we can do at the moment.
-- It is unlikely that people have actually done this as the documentation on adding new backend types will only go out in 5.3.
[CL 25039959 by paul chipchase in ue5-main branch]
#rb none
#jira UE-177956
#preflight
- Deprecate the config entry 'DepotRoot' and replace it with 'DepotPath'. The use of the term root was not clear and was causing people to think it was the name of the depot rather than the path to the root location to store payloads in.
- If 'DepotPath' is empty or missing, the value for 'ClientStream' will be used for it.
-- The 'DepotPath' can still be set when using streams, if the name of the stream is not a valid path (virtual streams for example.
[CL 25024791 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-180257
#preflight 6411cdbb5819afacaf06fbc9
- By default the value is true and the local ini file is used.
- Allows teams to opt out of loading settings from the locally saved ini file if they wish to.
[CL 24654053 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-170132
#rnx
#preflight 640f33c5363e9b40ab8cd7af
- Output the entire depot path when failing to download 'payload_metainfo' along with the address of the server.
- We ask the source control provider for the status and parse the p4 port from that as there is no way (currently) to get this info directly via the source control api, which currently works but is a bit flaky long term.
-- I am not sure if we want to keep the error reporting like this and so I am loath to make changes to the source control API unless I am sure we actually want the functionality.
- Removed duplicate code paths, there is now only one branch that will log an error.
- Removed call to FConnect as that does not do anything unless a client is set and we do not use one with this connection. (FConnect is mostly about client set up validation rather than actually connecting to the server since we always make a new perforce connection for each command anyway.
-- Instead we now check if the source control provider is available after we call Init, if this returns false then no connection was made.
- Documented the purpose behind calling ISourceControlProvider::Init
[CL 24613062 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-175158
#preflight 63f8cd34ae54ee4ce9ea207b
- We really should just read this value out of the users p4 environment settings, but that would require a source control api change and would need to be done in a source control agnostic way. Letting a project just define the name is a quick way to expose the functionality.
- At some point if we do extend the source control api then this option can be deprecated.
[CL 24400577 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-175157
#rnx
#preflight 63f8b493ae54ee4ce9de6b78
- This feature was requested so that a team could opt into submitting from their appdata or temp directories instead of the project saved directory (our default)
- Deprecated the option '-SubmitFromTempDir' as it could be replacated with "-WorkingDir=$(Temp)/UnrealEngine/VASubmission"
[CL 24399707 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-175189
#rnx
#preflight 63d27eae450d5cdd0b4df619
- We were raising a FMessageLog to print a warning, then using ::OnConnectionError to defer the FMessageLog::Notify call until the next editor tick so an easy fix is to pass the warning to ::OnConnectionError and piggyback onto the defferal if needed.
- The messages used to be errors but were demoted during development, now that FSourceControlBackend has SuppressNotifications option and can supress the notifications on a per project basis we can restore the messages to errors.
- When calling ::OnConnectionError we will send the message to FMessageLog if we are currently on the game thread, then defer the notification until next tick as we were doing so previously. The deferral is needed in case the notify comes during editor start up (which for a connection problem is likely) so that the notification appears at the correct time. If we are not on the game thread we should still print the message to the log file so that debugging is easier, only the FMessageLog part needs to be deferred to the GameThread. In this case we need to tell FMessageLog to not print to the log file to avoid duplicates.
[CL 23919185 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-174290
#rnx
#preflight 63c8fa22d15a37ae31487446
- Previously once the number of perforce connections allowed was reached we'd block all additional requests until an outstanding connection as been released at which point a random waiting thread would get the go ahead to make its own connection and so on. If the GameThread was making a request under these conditions the user could be left with a blocked editor while they wait for the GameThread to get a turn.
- This change allows FSourceControlBackend to create a semaphore that gives priority to the GameThread during contention so that the user gets the fastest possible response.
- FSemaphore now takes a bitfield of flags as a parameter in the constructor to allow us to specialize its behavior with the only support option being to prioritize the game thread.
- If the GameThread tries to acquire the semaphore and the prioritization option is enabled we use a slightly different path code path ::AcquireFromGameThread
-- Unlike the normal acquire code path, this path does not increment the count when waiting. This effectively reserves the next open slot for the calling thread.
-- Given that there is only one request on the GameThread at a time we don't have to check Counter once the event has been triggered. The even triggering means we know that one of the slots has been freed and we know we have already reserved that slot so we can just go ahead and return success at that point.
- Added a profile scope to acquire as that would've made the original problem easier to catch.
- Still need to look into replacing this with std::counting_semaphore once we can rely on c++ 20 support.
[CL 23771977 by paul chipchase in ue5-main branch]
Tested compiling fortnite, unrealeditor, lyra, qagame with non-unity/pch
#preflight 63635997876630122adeab9f
#rb none
[CL 22958990 by henrik karlsson in ue5-main branch]
#rb PJ.Kack
#jira UE-167578
#rnx
#preflight 635bdddd5b54febd38b7efd0
- Clean up the source control backend by removing the older code path that was ifdefed out and clean up the new path.
- FPerforceConnection no longer takes a TOptional<FSharedBuffer> but takes an optional pointer to a TArray of FSharedBuffer. When gathering files this should return one FSharedBuffer per file requested.
-- It might make more sense to attach the FSharedBuffer to the FP4Record but that would require some bigger changes and would be better suited to being some follow on work. All the code is private in the module and not exposed so it is fairly safe and easy to do these refactors later.
- Replaced the TArray buffer for gathering files with a purpose built class FP4DepotFile.
- When issuing a p4 print command with multiple files we will get one call to ClientUser::OutputStat followed by calls to either ClientUser::OutputText or ClientUser::OutputBinary until all of the file has been delivered, then we will get another call to ClientUser::OutputStat for the next file and so on.
-- Note with binary files we seem to get one final call to ClientUser::OutputBinary with zero size, which I assume is supposed to signal the end of the transfer, text files do not get this, they seem to just stop.
[CL 22888671 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-163093
#rnx
#preflight 63526abcae33b04ec1ee4a65
- The caller can now request more than one payload to be pulled froim the system at a time. In theory this will allow backends to pull data quicker.
-- The file system backend works with the new system but makes no attempt to take advantage as the backend is not intended for serious production use.
-- The DDC backend should be taking full advantage of the new batching API
-- Note that although the source control API does attempt to take advantage of batching, the internal source control API implementation is not doing so. This will be addressed in a future submit.
- This does make the pulling logic a bit more complicated in FVirtualizationManager as we need to deal with some payloads being found in the first backend, some in the second and some in the third etc.
-- To help deal with this a new class FPullRequestCollection has been added which abstracts a lot of complexity away.
[CL 22707955 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160942
#rnx
#preflight 6336f2ab5c2225fe5f6c08a5
- Removed the status enum from FPushRequest and replaced it with a class that represents the push result instead.
-- This lets us return more context about problems. At the moment it is only being used to return the filter reason if a payload is filtered out. In the future we could return per payload error messages etc.
- Now that we return more detailed info about the payload push we no longer report payloads that were already in the backend as being pushed which could cause some very misleading stats to be shown.
- Changed the backends to use the new results system.
-- Not all paths were correctly setting the state in the file system backend.
-- The source control backend does not set the result in all paths, which will leave the results in the "pending" state. Which will still mark the payload as not uploaded and so is safe to do.
- Removed the pushing of a single method from IVirtualizationBackend as it was only used in one place internally, so we might as well just change that code to call the batch push overload.
[CL 22279127 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-163834
#rnx
#preflight 632d98e4b4515b7e221654ec
### Virtualization
- At the moment there is no way in the editor UX to change the source control settings for the source control backend, so what ever settings are found in the global environment would be applied when the editor is first run and then saved to a config file under the <saved> directory. From this point onwards those settings would always be used when setting up the source control backend. If the settings were wrong, the first time that the editor was started, then we'd continue to use the incorrect settings even if the global enviroment was fixed. Making the backend work at that point would require that the user know about the local ini file and change the settings there.
- We cannot fix this by just ignoring the ini file as it has been requested that users can add their settings there in case they want to customize things, so we just need to avoid saving to it.
- Added documentation to the source control backends header file on how to set up the ini file if needed.
### Perforce
- FSourceControlInitSettings now accepts EConfigBehavior which describes how we want the source control provider to deal with its settings with regards to the ini file.
- Note that EConfigBehavior is not a bitfield as we do not wish to support only writing to the ini file, there is no point if it cannot be read from.
- When the source control provider is creatred we pass on the into from EConfigBehavior to the FPerforceSourceControlSettings to enable/disable saving and loading.
[CL 22163769 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-164284
#rnx
#preflight 632b182ffc7f1efbdf9638d9
- This is only useful if the entire team uses the same address for the perforce server
- When set, this will override the environment settings for P4PORT as well as override what ever is saved to SourceControlSettings.ini
[CL 22122737 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-163103
#rnx
#preflight 6318989c2b7fe03eb664e9f0
### VirtualizationSystem/VirtualizationManager
- Added an overload of ::Push taking just one FPUshRequest so that people don't have to keep adding MakeArrayView boiler plate when pushing a single request
- Change the order of the last two parameters for the raw ::Push call as this will group all of the payload specific parameters together and leave the target storage type at the end. It is unlikely that anyone is calling the older version but it has been deprecated for safety.
### IVirtualizationBackend
- The none FPushRequest overload of ::PushData is no longer virtual, it just converts the parameters to FPushRequest and calls that overload instead. In this way we now only have one pushing code path in our backends. We could probably look into removing this overload at this point (since the higher level IVirtualizationSystem will now convert all push requests into a FPushRequest form) but it is not considered worth it at the moment when the simple overload covers our needs.
- Removed EPushResult in favour of just returning true/false for the overall operation.If the caller needs a more detailed breakdown then they will have to use an overload that takes an FPushRequest over raw parameters.
-- At the moment FPushRequest does not contain a full breakdown of what happened, so with this submit we are effectively losing the ability to find out if the payload was already in the backends or not, however the batch version of push was already not returning this info so it is not a big loss. Fixing FPushRequest to return a better break down of what happened will be done in UE-160942
- Removed the none batch Push paths from the source control and ddc backends as they already supported batch pushing.
- File backend needed to be converted to supporting batch pushing, which is pretty much the same code as before except we need to iterate over the container of FPushRequests.
-- The backend does not early out on error as it tends to be quite fast. We might want to consider an official policy for the VA system, if we should early out of errors or not.
[CL 21907558 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-161973
#rnx
#preflight 63074424ae13a5a09865a281
- We could try force loading and enabling the plugin at this point but I am not a fan of the idea of overriding the set up of the target in a way that is not immediately obvious to the end user.
- The virtualization module needs to include the 'Projects' module to be able to access the plugin system directly and while editing the file I took the opportunity to clean up how the module is declaring its dependencies.
[CL 21684932 by paul chipchase in ue5-main branch]
#rb Devin.Doucette
#jira UE-161599
#rnx
#preflight 6303c8d65a5d4e4624e7bf52
- There are some use cases that require the VA system to be initialized and configured correctly but would prefer that the backend connections only run if absolutely needed (usually when a payload is pulled or pushed for the first time), this change provides four different ways of doing this:
-- Setting [Core.VirtualizationModule]LazyInitConnections=true in the Engine ini file
-- Setting the define 'UE_VIRTUALIZATION_CONNECTION_LAZY_INIT' to 1 in a programs .target.cs
-- Running with the commandline option -VA-LazyInitConnections
-- Setting the cvar 'VA.LazyInitConnections' to 1 (only works if it is set before the VA system is initialized, changing it mid editor via the console does nothing)
--- Note that after the config file, each setting there only opts into lazy initializing the connections, setting the cvar to 0 for example will not prevent the cmdline from opting in etc.
- In the future we will allow the connection code to run async, so the latency can be hidden behind the editor loading, but for the current use case we are taking the minimal approach.
-- This means we only support the backend being in 3 states. No connection has been made yet, the connection is broken and the connection is working.
-- To keep things simple we only record if we have attempted to connect the backends or not. We don't check individual backends nor do we try to reconnect failed ones etc. This is all scheduled for a future work item.
- If the connections are not initialized when the VA system is, we wait until the first time someone calls one of the virtualization methods that will actually use a connection: Push/Pull/Query
-- We try connecting all of the backends at once, even if they won't be used in the call to keep things simple.
- Only the source control backend makes use of the connection system. The horde storage (http) backend could take advantage too, but it is currently unused and most likely going to just be deleted so there seemed little point updating it.
- If we try to run an operation on an unconnected backend we only log to verbose. This is to maintain existing behaviour where a failed backend would not be mounted at all. This logging will likely be revisited in a future work item.
[CL 21511855 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-161296
#rnx
#preflight 62fe3ce73d3fb466b229bcc0
- There are some usecases that require the VA system to initialize the first time it is accessed (usually the first time we attempt to pull a virtualized payload) rather than be initialized in the program start up. This change provides three different methods to achieve this:
-- Setting the define 'UE_VIRTUALIZATION_SYSTEM_LAZY_INIT' to 1 in a programs .target.cs
-- Setting [Core.ContentVirtualization]LazyInit=true in the Engine ini file
-- Running with the commandline option -VA-LazyInit
- If we detect that the source control backend is being initialized on a background thread we do not try to run the FConnect operation. The backend will still work but this does reduce the potential error checking on initialization. This is done because the FConnect operation currently only works on the main thread and to change this would be a bigger work item than we can schedule at the moment.
- UE::Virtualization::Initialize functions now take a EInitializationFlags enum as a parameter. This enum allows the call to ignore all lazy init settings and force the initialization immediately. This is useful for programs like the Virtualization standalone tool which just needs to start the system when needed.
-- The call to ::Initialize in LaunchEngineLoop passes in None and does not ignore lazy initialization.
-- Calls to ::Initialize in the UnrealVirtualizationTool however all use EInitializationFlags::ForceInitialize and ignore lazy initialization settings.
- Fixed an odd bug in UE::Virtualization::Initialize where the error path (if the config file cannot be found) was using a different start up code path.
- Add asserts when assigning to GVirtualizationSystem to make sure that it is null. This is not 100% safe but should catch some potential threading issues, if any.
- Add an assert after lazy initialization (IVirtualizationSystem::Get) to make sure that GVirtualizationSystem was assigned a valid object.
- Improve how we check for legacy values in [Core.ContentVirtualization]. We now support multiple allowed values.
- Added a way to poll if a VA system has been initialize yet or not, this allows us to avoid initializing a VA system if one has not yet been created and we try to:
-- Dump VA profiling stats after cooking
-- Send VA stats to studio analytics
- Note that currently using lazy init loading will probably cause the VA statistics panel not to work, this will be fixed in future work where we will allow the panel to register for a callback when the system is initialized.
[CL 21467510 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160954
#rnx
#preflight 62fb9de9086f90bbc4394829
- The problem with the old logic was that we checked all of the payloads initially to see which needed to be submitted but if enough payloads were requested we would split the submission into a number of batches. If a payload was in more than once batch then it would be submitted in the first batch and then fail the MarkForAdd command in the subsequent batches. When we detected this error we would fail the virtualization process.
- In the long term we should probably eliminate duplicate requests at the highest level (FVirtualizationManager) so no backend has to worry about it, but we need this working now so it was easiest to add duplicate checks to the source control backend
-- Note that if another process submits a payload between the status check and the mark for add calls then we will also get errors, but that is true for a lot of our source control code.
- Now when pushing payloads we first iterate over the requests and create a unique list of requests to submit. This eliminates the possibility of duplicate payloads being submitted and failing the MarkForAdd command.
- We now record the push request state for each payload in a map and then only apply it to the original requests if the push succeeds.
-- We still only really have failed/success statuses and nothing recording "already in the backend" which is scheduled to be fixed elsewhere in the backlog.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 21404920 via CL 21408168 via CL 21408189 via CL 21408222
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v975-21357124)
[CL 21411940 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160943, UE-151671
#rnx
#preflight 62fa31bbae3edb54c979e53e
- By checking the state of the payloads first we can avoid loading and writing a payload to disk, just to find that it is already in the depot. If we are very lucky we might find that all of the payloads are already uploaded, in which case we can avoid creating the submission workspace as well.
- The lock to prevent too many concurrent p4 connections does cover a wider area of code than it did before, in theory we could relinquish the lock while creating the payload files for submission, but generally the time it takes to do that is much smaller than the source control operations themselves so there is probably not much point.
- Any request that we find is already uploaded we can just mark as a success. We then create an array of pointers to the requests that were not found and operate on that for the rest of submission.
- Note that we also split up the payload existence checks into batches with the same max size limit of submission, although checking if a payload exists should be much less taxing on the system I'd rather not risk overloading the perforce server where possible.
#robomerge FNMain
[CL 21386511 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-160083
#rnx
#preflight 62f64714185b21882a875427
- When this backend was written, it shared the source control provider interface with the rest of the editor, so if we were unable to restore the workspace it would likely break the editor in interesting and hard to diagnose ways.
- Given the scope for problems and the low change of the operatation I opted to make the error fatal.
- Now that the backend owned it's own source control provider interface, the worst thing that can happen is that future virtualization attempts will also fail, but the user will not be left with a non functioning editor. With this in mind there is no longer a reason to consider this a fatal failure.
[CL 21353746 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight 62f5f1a01e61d1ba0e6c893b
- Note that the older name 'SourceControl' will continue to work but will log a warning if used to notify the user to update their config files.
[CL 21352223 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#rnx
#jira UE-160619
#preflight 62f37c86b66d5d93133d67e3
- Based on work from Jess.Kube
### Virtualization
- The source control backend now takes an optional config option "ClientStream" which takes the name of the client stream to use.
- If a client stream is set then workspaces created for payload submission will use that and not provide a client-view mapping.
#ushell-cherrypick of 21195584 by Jess.Kube
### PerforceSourceControl
- Allow FCreateWorkspace to create workspaces with streams as well as classic workspaces cia FCreateWorkspace::SetStream.
- Add a method FCreateWorkspace::ClearClientViewMappings which will clear any client view mappings already added to the operation.
- If we detect that a FCreateWorkspace operation has both a stream set and client view mappings set then FPerforceCreateWorkspaceWorker will return an error. Perforce will allow us to creat a client spec with both entries, but will default to using the stream. Technically we could allow this too but it might cause unexpected behaviour to the caller. It is better to give a clear error and fail the workspace creation.
[CL 21316756 by paul chipchase in ue5-main branch]
