Commit Graph

1310 Commits

Author SHA1 Message Date
danny couture
6706fcda92 [ZenLoader/Misc]
- Prepare for EInternalObjectsFlag::AsyncLoading deprecation

#rnx
#jira UE-211622
#rb Francis.Hurteau

[CL 34672045 by danny couture in ue5-main branch]
2024-06-26 07:46:49 -04:00
paul chipchase
9d38539cb6 Clean up FBulkData deprecations
#rb Per.Larsson
#rnx

- Removed all code marked as deprecated for UE 5.2 and earlier.
- This removes the old async streaming api, which allowed the user to request that the bulkdata payload start streaming from disk and be stored internally by the bulkdata object to be used at some point in the future via a call to FBulkData::StartAsyncLoading which has been deprecated for a number of engine releases.
- Seems we missed deprecating ::IsAsyncLoadingComplete and it's associated flag 'EBulkDataFlags::BULKDATA_HasAsyncReadPending' when deprecating ::StartAsyncLoad so marked it for deprecation now along with code comments explaining why it's not used anymore.
- Since we are removing ::StartAsyncLoad, we can completely remove UE::BulkData::Private::StartAsyncLoad as it was that only place it was being used.
- ::FlushAsyncLoading only provided functionality if 'BULKDATA_HasAsyncReadPending' was set, which would only occur if ::StartAsyncLoad was called. Since we are not removing ::StartAsyncLoad we can remove the implementation of ::FlushAsyncLoading and deprecate it.
-- In turn this means we can remove UE::BulkData::Private::FlushAsyncLoad entirely as it was only being called by ::FlushAsyncLoading.
- With both  UE::BulkData::Private::StartAsyncLoad and UE::BulkData::Private::FlushAsyncLoad removed, we can remove FAsyncBulkDataRequests aswell as there is no longer any way for people to use it.
- Removed calls to ::IsAsyncLoadingComplete from the rest of the engine, it will always be true at this point.

[CL 34671288 by paul chipchase in ue5-main branch]
2024-06-26 06:53:39 -04:00
danny couture
ff6501af84 [ZenLoader]
- Fix assert when nodes are fired from the game thread

#rnx

[CL 34646433 by danny couture in ue5-main branch]
2024-06-25 13:16:29 -04:00
danny couture
ca31309174 [ZenLoader]
- Make UObject still being serialized invisible from postload functions
 - Fixes tons of issues where PostLoad could see uninitialized and RF_NeedLoad objects requiring additional code to filter these cases everywhere
 - Make the visibility logic reusable since it was duplicated at lots of places
 - Make sure behavior is the same whether async loading thread is active or not
 - Can be toggled with s.UseObjectVisibilityFilterForAsyncLoading (current default is false)
 - Will be made active by default soon

#jira UE-211622
#rb Francis.Hurteau

[CL 34637510 by danny couture in ue5-main branch]
2024-06-25 07:12:54 -04:00
matt peters
c05e086433 IncrementalCook: Add class metadata to the class schema, since class metadata (such as displayname) is often used in serialization functions that impact cooked packages.
This fixes the iterative false skip of UStateTree when a StateTreeNode's class's display name changes in c++, since the display name is incorporated into node names and saved into the cooked package.
#rnx
#rb Zousar.Shaker

[CL 34619700 by matt peters in ue5-main branch]
2024-06-24 16:42:44 -04:00
devin doucette
01cb852a23 Deprecated the old names and paths for TSharedString
#rb Zousar.Shaker
#rnx

[CL 34567121 by devin doucette in ue5-main branch]
2024-06-21 12:52:32 -04:00
kevin macaulayvacher
276d09f6df Remove all simple usage of REN_ForceNoResetLoaders from the codebase since the flag has been deprecated and currently does nothing. Simple in this case is direct use of the flag with no conditional logic. More complex uses were removed in another change.
#rnx
#rb Francis.Hurteau

[CL 34424068 by kevin macaulayvacher in ue5-main branch]
2024-06-17 11:55:02 -04:00
ben woodhouse
2ea61ce0c6 Switch to CSV_PROFILER_STATS instead of CSV_PROFILER for various profiling subsystems. This allows them to be compiled out when CSV_PROFILER_MINIMAL is defined.
As part of this change we also promote dynamic resolution and IO/PackageQueueDepth stats to Minimal since they're important for high level performance reporting.
Also fix up a few places that were redundantly using #if CSV_PROFILER around CSV macros.

#rb mickael.gilabert

[CL 34386798 by ben woodhouse in ue5-main branch]
2024-06-14 18:16:47 -04:00
devin doucette
36f5a24ddd Moved TSharedString from DDC into Core
#rb Steve.Robb

[CL 34340152 by devin doucette in ue5-main branch]
2024-06-13 10:40:59 -04:00
danny couture
8666ec1c53 [ZenLoader]
- Fix OnAssetLoaded so it is called in -game to match the behavior of the previous loaders

#rnx
#rb kevin.macaulayvacher

[CL 34306179 by danny couture in ue5-main branch]
2024-06-12 09:41:34 -04:00
kevin macaulayvacher
357c98fb83 - UObject::Rename will always remove a mismatching linker unless explicitly told not to via a new rename flag REN_AllowPackageLinkerMismatch. This is meant to be distinct from the deprecated REN_ForceNoResetLoaders as the intent is inverted: There are few reasons to not want to reset loaders when there is a mismatch between package and linker, and when there is no mismatch, we do not want Rename to implicitly resetloaders (we no longer do as of 33136565). The only use case where mismatched linkers is desired is when objects are _temporarily_ renamed to belong to a staging package and desire to have the object continue loading when the object is moved back to the matching linker package (commonly done during blueprint reinstancing). In such a case, the new flag acknowledging the intentional mismatch is meant to be used.
- Any code causing a linker mismatch needs to explicitly clear loading flags for their object to avoid asserting. This is to make the intention clear that rename will make loading correctly impossible when moving the object to a different linker unintentionally. So either clear the loading flags of the object (i.e. finish loading via load calls such as ConditionalPostLoad) or forcibly allow mismatching via the new flag.

- When marking an export as invalid in FLinkerLoad::InvalidateExport() we now also clear loading flags since it is wasteful to load invalid objects.

- Exports marked as invalid are no longer reset to be valid when the linker is reset for the object instance. This is to prevent reloading invalid objects when re-using a linker already resident in memory if a subsequent package load is requested.

- When UStruct upgrades UFields to FFields, the UStruct objects are marked as invalid to prevent reloading the objects unnecessarily

- Fixes an issue where since Rename can clear the linker for mismatched types, any calls to FLinkerLoad::InvalidateExport needs to be moved before the rename operation to be effective.

- CollectUnreachableObjects will no longer mark unreachable objects as invalid in the linkerloader. That code would mark objects as invalid until the linker is destroyed which incorrectly assumed would happen soon. If however a load for the same package occurs before the linker is destroyed, the in-memory linker would be re-used keeping the marked exports as invalid so those exports would not be loaded (even though they should have been).

#jira UE-212466 UE-214849
#rb Francis.Hurteau, Michael.Galetzka
[RN] UObject::Rename will always remove the renamed object's linker if the rename moves the object into a different package. As a result, any renames occuring while an object is loading will now assert instead of leading to hard to diagnose crashes long after the Rename call has completed. If renaming to a different package is intentional and the linker should not be cleared, the new flag REN_AllowPackageLinkerMismatch can be used to prevent clearing linkers on the object even though the linker is for a different package than the package has been renamed to be part of.

[CL 34304935 by kevin macaulayvacher in ue5-main branch]
2024-06-12 08:47:36 -04:00
matt peters
55c4daaff8 IncrementalCook: Add TObjPtr references made from Linker::PreLoad and UObject::PostLoad to the dependencies for the package of the object being pre or post loaded.
#rnx
#rb Zousar.Shaker

[CL 34161632 by matt peters in ue5-main branch]
2024-06-06 12:30:55 -04:00
devin doucette
118ab112d0 Deprecated GetSerializeContext and SetSerializeContext on FArchive
Implementations have been effectively using the thread context and these changes make that the only available context.

#rb kevin.macaulayvacher

[CL 34011179 by devin doucette in ue5-main branch]
2024-05-30 16:02:03 -04:00
robert millar
df1e10047d Add new AUTORTFM macros which can be followed by braced blocks to make them work better with the debugger
e.g. UE_AUTORTFM_OPEN2{ ... code ... };

#rb neil.henning

[CL 33869184 by robert millar in ue5-main branch]
2024-05-23 14:22:32 -04:00
matt breindel
00b9d5b26b Trivial TSAN fix to make a thread status bool atomic.
#jira UE-215091
[REVIEW] [at]*kevin.macaulayvacher
#rnx

#rb kevin.macaulayvacher

[CL 33809854 by matt breindel in ue5-main branch]
2024-05-21 17:05:52 -04:00
kevin macaulayvacher
d3fb5a7269 Accelerate CreateLinkerExports by replacing the FPublicExportMap's custom container with a TMAp so we may fetch ObjectIndicies via their ExportHash. This dramatically improves performance for both addition and removal when dealing with many exports. Iteration of exports is moderately improved when exports have been removed.
This fixes:
- The previous container relied on a fixed array of sorted indices. Each insertion would mean memmov'ing chunks indices and values
- Removing elements doesn't actually shrink the table (that's mostly fine) but instead adds a sentinel value. This means anytime a value is looked, for a sentinel value comparison is needed. When fetching all publicexports, we need to walk the whole table despite it not being uncommon in editor for exports to be invalidated and can be removed from iteration entirely.

When loading a UMap with 2M exports, we now take 25s compared to 44m. This usecase comes from a real customer umap.

#jira UE-201114
#rb danny.couture, Francis.Hurteau

[CL 33723462 by kevin macaulayvacher in ue5-main branch]
2024-05-17 10:16:53 -04:00
kevin macaulayvacher
ab2a86fed0 [Backout] - CL33602286
Backout until we can determine all potential knock-ons

[FYI] kevin.macaulayvacher
Original CL Desc
-----------------------------------------------------------------
Fixes two issues in zenloader around package export invalidation:
- Previously CollectUnreachableObjects would mark GC'd sub-objects as invalid. This could break future loads of the package while the package (but not all its sub-objects) are still in memory. (UE-213934)
- Previously importing a package which may convert/upgrade it's exports could result in the old objects lingering. These lingering objects might cause issues during cook/saving (UE-213910, note while this bug is specifically for -nozenloader, the changes in this CL fix the same issue for -zenloader)

To fix the above, when a package is finished loading we synchronize the FAsyncPackage2 exports with LFLinkerLoad's exports and ensure the FAsyncPackage2 contains at least the same invalid/bExportLoadFailed=true exports. These exports and any export that is a sub-object to an invalid export, are stored in the GlobalImportStore. When we next directly load or import a package, we will filter out any invalid exports. Note, we synchronize the invalid exports at the end of package loading because Preload and PostLoad codepaths have opportunities to alter exports, so we wait for those to complete before we do any recording. When a package is removed from memory, we remove all invalid exports from the GlobalImportStoreto prevent the InvalidExport map growing unbounded.

CollectUnreachableObjects no longer marks GC'd objects as invalid, and instead we rely on the logic described above. This prevents us from incorrectly marking valid objects, we should reload when later requested, as invalid.

Two tests were added to validate the two issues above, and in doing so the existing AsyncLoading test files that were scattered in the test directory have been moved to a test specific directory for clarity

#jira UE-213934, UE-213910
#rb danny.couture, Francis.Hurteau


#virtualized

[CL 33659845 by kevin macaulayvacher in ue5-main branch]
2024-05-15 11:24:26 -04:00
kevin macaulayvacher
2d0b5b407a Fix use of potentially unset optional LinkerLoadState in FAsyncPackage2::SyncAndStoreInvalidExports.
#rnx

[CL 33613103 by kevin macaulayvacher in ue5-main branch]
2024-05-13 20:15:36 -04:00
kevin macaulayvacher
6afb852e5c Fixes two issues in zenloader around package export invalidation:
- Previously CollectUnreachableObjects would mark GC'd sub-objects as invalid. This could break future loads of the package while the package (but not all its sub-objects) are still in memory. (UE-213934)
- Previously importing a package which may convert/upgrade it's exports could result in the old objects lingering. These lingering objects might cause issues during cook/saving (UE-213910, note while this bug is specifically for -nozenloader, the changes in this CL fix the same issue for -zenloader)

To fix the above, when a package is finished loading we synchronize the FAsyncPackage2 exports with LFLinkerLoad's exports and ensure the FAsyncPackage2 contains at least the same invalid/bExportLoadFailed=true exports. These exports and any export that is a sub-object to an invalid export, are stored in the GlobalImportStore. When we next directly load or import a package, we will filter out any invalid exports. Note, we synchronize the invalid exports at the end of package loading because Preload and PostLoad codepaths have opportunities to alter exports, so we wait for those to complete before we do any recording. When a package is removed from memory, we remove all invalid exports from the GlobalImportStoreto prevent the InvalidExport map growing unbounded.

CollectUnreachableObjects no longer marks GC'd objects as invalid, and instead we rely on the logic described above. This prevents us from incorrectly marking valid objects, we should reload when later requested, as invalid.

Two tests were added to validate the two issues above, and in doing so the existing AsyncLoading test files that were scattered in the test directory have been moved to a test specific directory for clarity

#jira UE-213934, UE-213910
#rb danny.couture, Francis.Hurteau


#virtualized

[CL 33602309 by kevin macaulayvacher in ue5-main branch]
2024-05-13 08:38:00 -04:00
danny couture
6160748181 [ALT]
- Fix TSAN warning on FApp::IsGame() changing on GT while creating linkers by moving the check as the very last condition where it is highly unlikely to ever get reached in editor (the only place it can change).
 - Since the value should never changes in -game and GIsEditor should never changes in editor we should never get into a state where that value dynamically affects the outcome of the loading but TSAN complained because we read it in all cases.

#jira UE-175442
#rb kevin.macaulayvacher

[CL 33536251 by danny couture in ue5-main branch]
2024-05-08 20:21:44 -04:00
danny couture
9e9f162ab2 [ZenLoader]
- Improve stall detector by monitoring activity on ALT when GT is waiting.

#rb kevin.macaulayvacher

[CL 33509912 by danny couture in ue5-main branch]
2024-05-08 08:28:44 -04:00
danny couture
c36a119ff8 [ZenLoader]
- Increase stall detection as current default can trigger in apparently normal case.

[CL 33482098 by danny couture in ue5-main branch]
2024-05-07 08:56:36 -04:00
paul chipchase
8b328403c0 Improve error reporting when making an invalid bulkdata stream request.
#rb Per.Larsson
#rnx

- If a user tries to make a read request from FBulkDataBatchRequest that goes past the end of the bulkdata's payload we will eventually assert, but only at the point of reading which is often quite far away (context wise) from the code that initiated the request.
- Previously we were asserting if the user made a read request that exceeded the size of the bulkdata payload but this did not take into account reading from an offset.
- Fixed the assert to take offset into account so we now asset on invalid reads at the point that the read is first requested. In addition I have added more debug info to the assert making it easier to tell from the log file what was incorrect about the read request.
- Changed the assert to an ensure because although a too large size usually indicates a higher level bug, the lower level file system code will generally prevent us from reading past the end of the payload and return the data up until the end of the payload. So far in every case where we have seen this happening, reading to the end of the payload was acceptable and the higher level code worked. An ensure will allow the problem to remain visibile to programmers but not block content teams should the error make it to production.
-- This logic needs to be revisited as we should be mroe consistent with async and sync file reads and how we return errors, ideally allowing the higher level code to decide if a failure is important or not as we do not really have enough context at lower level to decide.

[CL 33479013 by paul chipchase in ue5-main branch]
2024-05-07 02:50:41 -04:00
matt peters
4ede2ab9b2 FUnversionedStructSchema::Create: Add CA_ASSUME to fix static analysis warning C6011: Dereferencing NULL pointer 'Struct'.
#rnx
#rb matt.peters
#rbself trivial

[CL 33473960 by matt peters in ue5-main branch]
2024-05-06 18:21:13 -04:00
matt peters
c66ea76942 IncrementalCook: Extend UStruct's schema hash to include the name of the class or struct. Type names are referred to by name in package headers so if the struct name changes then the schema hash needs to change as well. This fixes the false positive iterative skips that occurred when /Script/Engine.PerPlatformProperties was renamed to /Script/Core.PerPlatformProperties.
#rnx
#rb Zousar.Shaker

[CL 33467274 by matt peters in ue5-main branch]
2024-05-06 15:56:34 -04:00