#rb Per.Larsson
#jira UE-141304
#rnx
#lockdown Mark.Lintott
#preflight 61fd3cccb5092d45ad18c953
- Not all backends gain from pushing payloads in batches. In this case the backend can opt not to override the default implementation which will just iterate over the batch of payloads and then push each one individually.
- However when the default path was used, FPushRequest::Status was not updated after the push was performed, which resulted in each request returning the default (failed) status.
-- The backends that use the default path are not enabled in any of our internal projects so this was not noticed.
- So even though the payloads were pushed, the virtualization process would log and error and block any submit being performed.
-- Subsequent submits of the same packages would work because we would note much earlier that the payloads are already stored in the backend and the broken code path would never run.
- Added fall through comment as required by the coding standards
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18863318 in //UE5/Release-5.0/... via CL 18863334 via CL 18863548
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v910-18824042)
[CL 18863566 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#rnx
#preflight 61fabeb8ad2ae6c3b763a2b9
- Renamed to ::QueryPayloadStatuses since the output data is in the form of an array of FPayloadStatus
- The method now returns EQueryResult rather than bool, which has 'Success' as value 0, all other values indicate an error.
-- This can be expanded in the future if we start to provide more info from backends as to what actually failed.
- FPayloadStatus has been cleaned up so that 'Partial' is not 'FoundPartial' to give a better indication that the payload was found in at least one storage backend, but not all of them.
[CL 18861036 by paul chipchase in ue5-main branch]
#rb Per.Larsson, Devin.Doucette
#jira UE-133497
#rnx
#preflight 61f835491c5ac5523462810a
- A lot of files have been touched but most of this is changing FPayload::IsValid to !FIoHash::IsZero, the main changes are in EditorBulkData/EditorBulkDataTests
- The only difference between FPayloadId and FIoHash is that the former considered the hash of an invalid or empty buffer to be 'invalid' too where as FIoHash would return a valid hash
-- To keep behaviour the same, we only hash payloads in EditorBulkData using a utility method ::HashBuffer which will not hash empty or invalid payloads and return a default FIoHash instead.
-- Unit tests have been extended to prove that the behaviour has not changed.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18806362 in //UE5/Release-5.0/... via CL 18808527 via CL 18821790
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v908-18788545)
[CL 18822154 by paul chipchase in ue5-main branch]
#rb PJ.Kack
#rnx
#jira UE-136758
#preflight 61f3e3626b5aea38e5b4cab7
- Package filtering was disabled when the api was changed to take batches of payloads to push to the backend as the context per payload had to be changed to a raw FString rather than FPackagePath.
- We now convert each context to a FPackagePath and if it is valid test against filtering, if it is not valid we assume that the payload did not come from a package.
- Still tempted to move filtering out of virtualization manager entirely and move it to PackageSubmissionChecks, but that can always be done at a later date.
- Retested the filtering system, plus the ability to add filters to the config files for GameFeature plugins.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18776038 in //UE5/Release-5.0/... via CL 18777356 via CL 18777639
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v903-18687472)
[CL 18777650 by paul chipchase in ue5-main branch]
#rb PJ.Kack
#rnx
#jira UE-140366
#preflight 61f2a6f752396dbfeeede332
- Add a way to poll the virtualization system to see if pushing to a backend storage type is possible or not.
- If we cannot push to persistent backend storage when submitting packages to source control we just log in verbose mode and return rather than giving an error. Submitting a non-virtualized package is no longer the problem it once was.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18769119 in //UE5/Release-5.0/... via CL 18769125 via CL 18769151
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v903-18687472)
[CL 18769154 by paul chipchase in ue5-main branch]
#rb none
#jira UE-140126
#rnx
#preflight 61efbb8e8e5c8d4eaabb987c
- FScopedStatsCounter can only add a single hit/size in it's destructor, so a for loop calling ::AddHit on it was just overwriting the values each time.
- Use TrackCyclesOnly so that 'Timer' only adds the time taken to the stats.
- Now the for loop adds hits to the stats manually for each successful request.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18720805 in //UE5/Release-5.0/... via CL 18720806 via CL 18720836
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v903-18687472)
[CL 18720839 by paul chipchase in ue5-main branch]
#rb PJ.Kack
#rnx
#jira UE-139708
#preflight 61eaaedfc92021e535b6d1e0
- The goal is to show a clear separation between the bulkdata object and the virtualization system.
- NOTE that all of these classes were added during 5.0 development so we are just renaming and moving them, no deprecation paths.
- Renamed FVirtualizedUntypedBulkData to FEditorBulkData
-- Also removed the derived templated types, they were never actually used
- Renamed FVirtualizedBulkDataReader to FEditorBulkDataReader
- Renamed FVirtualizedBulkDataWriter to FEditorBulkDataWriter
- Moved the renamed classes to the UE::Serialization namespace from UE::Virtualization
- Renamed the files of the renamed classes where required.
- Replaced use of LogVirtualization with LogSerialization.
- Renamed defines prefixed with VBD_* to UE_ and make sure they are undefed by the end of the cpp
- Edited unit tests to show up under "System.CoreUObject.Serialization.EditorBulkData.*"
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18688778 in //UE5/Release-5.0/... via CL 18688787 via CL 18688810
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v903-18687472)
[CL 18688823 by paul chipchase in ue5-main branch]
#rb PJ.Kack
#jira UE-139348
#rnx
#preflight 61e6742a3778a195dea02199
### PackageTrailer
- We no longer store the access mode for all payloads in the trailer. Instead each payload entry in the lookup table will store the access mode for that specific payload. This allow us to store many payloads of different access types in the same trailer.
- Note that this format change is currently versioned (going from 0 to 1) and is compatible with older version.
-- To allow this, the << operator has been removed from FLookupTableEntry and replaced with a ::Serialize method that takes the version of the package trailer header as an input so that it can run it's own versioning checks.
- When loading trailers with version 0 we apply the access mode to all non virtualized payload entries in the lookup table to reproduce the single access mode for the entire trailer.
- A FPackageTrailerBuilder can now be created directly from an existing FPackageTrailer that will reference all of the trailers local payloads via the new static method ::CreateReferenceToTrailer
- Removed FPackageTrailer::TrySave as it is no longer used when creating a trailer for the editor domain.
- Renamed FPackageTrailer::LoadPayload to ::LoadLocalPayload to make it clear the type of payload that can be loaded.
-- Someday the package trailer should be able to load payloads of any type.
### VirtualizedBulkData Serialization Changes
- We now record if the bulkdata's payload is stored in a package trailer or not via the 'StoredInPackageTrailer' flag, which makes it easier to identify critical errors in serialization (the trailer is missing for example)
-- Storing this info as a flag is now possible as the design for the trailer has changed and a fully virtualized package will still contain a trailer.
- We now only write out OffsetInFile if the payload is not stored in a FPackageTrailer, if the bulkdata object does use a package trailer via the new flag then we take the offset from the trailer instead.
- We also no longer write out a path when saving a payload as referenced to the editor domain, it is assumed that the trailer will know where the workspace domain data is (namely the owning package path)
-- Due to the 'StoredInPackageTrailer' only being added now, this means that packages already saved with a package trailer will work with this code logic as those packages will have serialized the offsets to disk, but won't have the new flag and so will serialize the offsets when loading until re-saved. (worth noting that nobody should encounter this as the package trailer system is disabled by default)
- The loading code is a lot easier to read now and a lot of the branches have been removed although we still have a few extra checks for older formats that might have been saved during development.
- When loading the only real consideration we have is if the payload is in the trailer or not.
- Future Work:
-- If we are not writing to the trailer then we use the legacy serialization system which appends the bulkdata to the end of the package. This is used in two cases, 1) When the package trailer system is disabled 2) If the payload is updated in postload and we are writing out the package to the editor domain as the package trailer does not support writing out local payloads outside of the workspace domain at the moment. The trailer should handle this itself and not rely on the legacy serialization system so that we can remove it.
-- Note that if a payload is stored by reference, we are not adding it explicitly to the payload trailer and we are relying on the trailer builder already containing the reference information when it is created in SavePackage. We do however run an assert to ensure that the reference does exist in the trailer but it might be worth revisiting this.
### VirtualizedBulkData Misc
- The member 'PackageSegment' is now initialized in the constructor to EPackageSegment::Header.
- Moved ::GetPackagePathFromOwner from being a method to a utility function as it accessed no members.
- Validating the payload for saving during serialization has been moved from ::Serialize to it's own method ::TryPayloadValidationForSaving.
### PackageSubmissionChecks
- Added an error message if we detect referenced payloads in a trailer we are trying to virtualize as this condition should never occur. If we encounter it, we give a user facing error message and fail the submit process. It might be worth demoting this to an assert instead as it is not an error the user is expected to see.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18637906 in //UE5/Release-5.0/... via CL 18637926 via CL 18637929
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v899-18417669)
[CL 18637932 by paul chipchase in ue5-main branch]
#rb PJ.Kack
#jira UE-136612
#rnx
#preflight 61e589b7904123989a1817cc
- If a perforce connection issue is encountered when starting up the editor we will now print an error to the message log and then on the first tick of the editor display a notification to get the users attention. It will no longer result in a fatal error.
- Add a new category "Asset Virtualization" to the message log so that we can start to improve the feedback we give to end users rather than only reporting problems via UE_LOG. This also introduces a new localization text name space.
-- I will need to go over a lot of the virtualization modules error handling and move it to use the message log instead.
- When the source control backend first starts up we now specifically check if the connection works and if not try to give the user feedback on how to fix it.
-- Ideally we'd provide a login dialog for the user to fix the problem in place but due to how early in the start up process we need to be able to access the virtualization backends this is quite tricky.
- In the future we will build the concept of a backends connection into a formalized concept in the system along with better error handling. At this point FSourceControlBackend::OnConnectionError should be moved to general use for all backends. this will allow us to provide much better status info to the user.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18637036 in //UE5/Release-5.0/... via CL 18637043 via CL 18637054
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v899-18417669)
[CL 18637614 by paul chipchase in ue5-main branch]
The structure with a value and attachments is an artifact of an early design where the record *was* a compact binary package. That design changed months ago and it now simplifies the cache record significantly to make it a collection of values without specific "value" and "attachment" types.
#rb Zousar.Shaker
#rnx
#preflight 61d8b94ed17842e547def37a
#ROBOMERGE-AUTHOR: devin.doucette
#ROBOMERGE-SOURCE: CL 18553971 in //UE5/Release-5.0/... via CL 18553976
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v899-18417669)
[CL 18553977 by devin doucette in ue5-release-engine-test branch]
A payload was conceptually a value with an ID. That has been formalized by removing the ID from the payload and having separate FValue and FValueId types. This separation cleans up the API in a few areas, and provides a more natural path to providing a basic key/value API.
#rb Zousar.Shaker
#rnx
#preflight 61d704c04c252480ca284d61
#ROBOMERGE-AUTHOR: devin.doucette
#ROBOMERGE-SOURCE: CL 18531844 in //UE5/Release-5.0/... via CL 18531856
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v899-18417669)
[CL 18531864 by devin doucette in ue5-release-engine-test branch]
#rb PJ.Kack
#rnx
#preflight 61b79447f807c623046e91a5
### FPackageTrailer
- FPackageTrailerBuilder::Create now renamed to ::CreateFromTrailer to make it a bit clearer.
- Added 'AccessMode' to the file format documentation.
- FPackageTrailerBuilder no longer tracks infomation about the trailer once it is written to disk. Instead when writing to disk we will create a valid FPackageTrailer as part of the serialization. This will be passed to callbacks and can be queried by virtualized bulkdata.
-- Since the builder no longer tracks data after creating the trailer, we can remove checks from methods like ::AddPayload.
-- Removed FPackageTrailerBuilder::FindPayloadOffset and added a version to FPackageTrailer instead.
- Changed a number of methods to take FPayloadId by reference rather than by value.
### VirtualizedBulkData
- Replaced areas of code responsible for updating previously written out values with the utility function ::UpdateArchiveData to help with code readability.
### Misc
- FLinkerSave no longer creates a FPackageTrailerBuilder by default, instead one should be added to the FLinkerSave if we want to build a trailer for the package being saved.
- FLinkerLoad now considers 0 to be an invalid value for PayloadTocOffset in addition to INDEX_NONE, so we now check if the value is > than 0. This is because when FPackageFileSummary objects are created, all members are set to zero and we know that a FPackageTrailer cannot be 0 and the rest of the package still exist so we can safely consider it invalid.
- SavePackage/SavePackage2 now create the FPackageTrailerBuilder when the linker is created if the package trailer feature is enabled.
-- We now always call BuildAndWriteTrailer and then only skip the creation of the trailer if the builder is missing or conditions are not met.
-- If we do not write out the trailer, we make sure to set PayloadTocOffset in the FPackageFileSummary to INDEX_NONE as it is clearer than leaving it at the default 0.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18453324 in //UE5/Release-5.0/... via CL 18453328
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v898-18417669)
[CL 18453333 by paul chipchase in ue5-release-engine-test branch]
#rb trivial
#jira UE-136515
#rnx
#preflight 61b21e3b764790bee6bdb8b3
- This backend is still most likely going to be deleted before 5.0 ships but just in case it is not I am doing the renaming now to keep in step with the names we are using for Jupiter when communicating with externals.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18419810 in //UE5/Release-5.0/... via CL 18422457
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v897-18405271)
[CL 18422718 by paul chipchase in ue5-release-engine-test branch]
#rb PJ.Kack
#jira UE-136126
#rnx
#preflight
### VirtualizationSystem
- Added a new overload for Push to VirtualizationSystem that takes an array of FPushRequest, which is a new structure representing a single payload request.
- Filtering by package name is currently disabled, this is because the API has been forced into changing and passing the package name in via a FString rather than FPackagePath which means we would need to be more careful. This will be done in a future submit.
- The backend interface has been extended to also have a batch version of PushData, by default this will attempt to submit each request one at a time so payloads don't have to try and implement a batched version if there is no need.
- The context being passed with a payload when being pushed has been changed from FPackagePath to FString due to include order issues, as the FPackagePath lives in CoreUObject and the API for virtualization lives in Core. Additionally in the future the payloads might not be owned by a package (there is nothing specifically enforcing this) so the context being a string makes more sense.
- NOTE: Due to the context change we currently no longer support the filtering feature, which allows for payloads belonging to packages under specific directories to be excluded from virtualization. This is something that will be solved in a future submit.
### SourceControlBackend
- Now that we can submit multiple payloads in the same submit, the CL description has been changed slightly. We will now print a list of payload identifiers -> the package trying to submit that payload. This will only tell the users which package originally caused the payload to submit. If a user submits a new package at a later date that contains the same payload we will not be updating the description.
### PackageSubmissionChecks
- Converted the submission process to use the new batch push operation in VirtualizationSystem.
-- This means that we do a single push and then have to update the package trailers to convert the now pushed payloads from local to virtualized.
- Added new define UE_PRECHECK_PAYLOAD_STATUS that makes it easy to toggle off the checks to see which payloads need to be submitted to the persistent backend. This is useful to test if it actually helps speed up the overall operations or if it is faster to just perform the batch push operations on all payloads and check the return values.
-- The hope is that over time the submission processes will become fast enough that we can remove the precheck.
- Fixed up logging to not always assume more than one package or payload.
### General Notes
- Errors and logging is now a bit more vague as we often not just report that X payloads failed etc rather than specific payload identifiers. This probably doesn't affect the user too much since those identifiers as fairly meaningless to them anyway.
- The source control submission could be further optimized by first checking the status of the files in thge depot and only then creating/switching workspace etc.
- As currently written, we need to load all of the payloads into memory, then the backends will do what they need (in the case of source control this results in the payloads being written to disk then submitted) which could create quite a large memory spike when submitting a large number of packages.
-- One solution would be to change the batch push API to take a "payload provider" interface and have the payloads requested as needed rather than passing in the FCompressedBuffer directly. This would let us immediately write the payload to disk for submission then discard it from memory, preventing larger spikes. Although it could cause overhead if there are multiple backends being submitted to. Internally we are unlikely to have more than one backend per storage solution so maybe we should just make it a config option?
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18403735 in //UE5/Release-5.0/... via CL 18403737
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v896-18170469)
[CL 18403738 by paul chipchase in ue5-release-engine-test branch]
#rb PJ.Kack
#rnx
#preflight 61af1090c6650f98a97782a4
- The source control backend will now attempt to create the temporary workspace as a partitioned workspace by default.
- This can be overriden via the config file when defining the virtualization graph.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 18392795 in //UE5/Release-5.0/... via CL 18392806
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v896-18170469)
[CL 18392807 by paul chipchase in ue5-release-engine-test branch]
Requiring the use of a separate reader type makes it more likely that readers will be reused, and makes it easier to audit reader usage going forward. Reusing readers is desirable to reduce the number of large temporary allocations made during partial decompression of a buffer.
- Added FCompressedBuffer::Save(FArchive&) and renamed FromCompressed(FArchive&) to Load(FArchive&).
- Added FCompressedBufferReaderSourceScope to set a buffer source within a scope.
- Added proper bounds checks to FNoneDecoder.
- Store the header checksum on the decoder context to allow raw blocks to be reused across sources.
- Decode the header on the fly to avoid a temporary header allocation when the header is in contiguous memory.
#rb Zousar.Shaker
#rnx
#preflight 61a98d53800738dbfbc84c73
#ROBOMERGE-AUTHOR: devin.doucette
#ROBOMERGE-SOURCE: CL 18382211 in //UE5/Release-5.0/... via CL 18382310
#ROBOMERGE-BOT: STARSHIP (Release-Engine-Staging -> Release-Engine-Test) (v896-18170469)
[CL 18382377 by devin doucette in ue5-release-engine-test branch]
