Skip to content

Rework asynchronous asset registry #391

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.

Sign up for GitHub

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

Jump to bottom
Draft
Helco wants to merge 41 commits into
base: master
Choose a base branch
from
Draft

Rework asynchronous asset registry #391

Helco wants to merge 41 commits into from

Conversation

@Helco
Copy link
Owner

@Helco Helco commented Oct 30, 2024 •
edited
Loading

Some time ago I theorized that the asset registry has a race condition regarding the order of apply actions (see #344). Also during development of an asset validation command I discovered a very real deadlock (see #386) which can happen when secondary assets fail to load and some (in hindsight) shoddy locking mechanisms in Asset.PrivateLoad

I now want to:

  • Add sufficient amount of unit tests before reworking
  • Rework locking mechanism of Asset.PrivateLoad
  • Add proper asynchronous loading of assets (for parallel tools) alongside the synchronous interface (for gameplay)
  • Prevent any synchronous waits outside of the main thread
  • (at least attempt to) Prevent any asynchronous waits without cancellation tokens registered
  • Workaround STA not being available on Linux (causing NUnit to not be single-threaded)
  • Fix smaller bugs and refactorings when convenient, e.g.:
    • AssetHandle equality
    • AssetHandle.Registry in regards to local registries and handle scopes
    • Inspecting all exceptions from secondary asset loads or multiple primary asset loads
    • Prevent recursive asset dependencies
  • Misc TODOs:
    • Dispose stored secondary handles in all game assets
    • Add tests for AssetHandleScope
    • Add tests for IAssetRegistryStats
    • Retest game before merge
    • Retest validation command before merge

@Helco Helco force-pushed the async-asset-registry branch from 3720c47 to ea240f8 Compare November 6, 2024 06:38
@Helco Helco mentioned this pull request Dec 18, 2024
Closed
Helco added 28 commits April 3, 2025 10:47
@Helco
Fix pedantic IDE warnings in asset registry
01fd4cc
@Helco
Add first two test cases
028cfa9
@Helco
Fix AssetHandle equality and add more test cases
288e86b
@Helco
Add even more test cases
3d2ac04
@Helco
You guessed it.
7bbee8e 
More test cases (with two currently failing)
@Helco
Fix WaitAsyncAll not checking disposed handles
4421884
@Helco
Add TestCases for applying async assets
9bbf0f6
@Helco
Add first batch of secondary asset tests
99443de
@Helco
Couple more test cases
d2e6b5a
@Helco
More secondary asset tests
be6b9a6
@Helco
Fix apply action not called when Load while loaded asynchronously
7d304b2
@Helco
Improve tests for async apply
65dd3ad
@Helco
Use TryWrite instead of WriteAsync and lock assets on registry disposal
5f9807f
@Helco
Replace synchronous wait on task with direct exception dispatch
c842097
@Helco
Use Assert.Inconclusive instead of Ignore
0c7bb45
@Helco
Add low-then-high/sync tests
7d20a8d
@Helco
Fix asset disposal on primary error
cd2fd0d
@Helco
Add secondary error tests and fix exceptions being swallowed by Task.…
8dd8de1 
…WhenAll
@Helco
Now I am become deadlock, the breaker of tests
c079ad4
@Helco
Asset.Load is now synchronous.
b05ca78 
This breaks everything eeeveen further
@Helco
Unfortunately fix all tests by adding async asset loading
66698f4
@Helco
Let the deadlocks surface in the tests instead of crashing the test p…
3fdb2c6 
…rocess with a somewhat unrelated exception
@Helco
ci: Add timeout to test steps
abb4de9
@Helco
CI should have failed...
c89871c
@Helco
Lost confidence in this approach. Let's try that again
446bb3c
@Helco
Revert "Lost confidence in this approach. Let's try that again"
c4cfff1 
This reverts commit 990053f.
@Helco
Broke everything in another way with slightly simpler asset locking
267e115
@Helco
Remove STA apartment
2b99280
Helco added 12 commits April 3, 2025 10:47
@Helco
Fix some problems
0f01fdf 
- Not queuing low assets
- Not dereferencing assets
- Not queuing apply actions
- Readd STA and postpone fixing Linux CI
@Helco
Loosen expected exception type for a couple tests. Not particularly p…
66c1b91 
…retty...
@Helco
Let assets keep an internal reference to all secondary assets
37648da
@Helco
Add SynchronizedTestFixture as workaround for Linux STA
26a638f
@Helco
Incremental builds are not working, explicitly do not build ontest af…
08480a2 
…ter build
@Helco
Add reload tests - because they consistently break everything again
051a4c9
@Helco
Suspiciously a bit more accurate test handling passes (at least on Wi…
13461c7 
…ndows)
@Helco
Test and fix asset registry disposal during asset load
034b3a9
@Helco
Fix some asset errors not being reported as Aggregate in WaitAsyncAll
7a2cd9b
@Helco
Slightly more robust handling of statelock waiting and finalizing the…
1b7b162 
… load tasks
@Helco
Fix exception type on statelock cancellation in WaitAsyncAll
277b1e2
@Helco
Check for invalid state transitions
39a95ae
@Helco Helco force-pushed the async-asset-registry branch from 1bd8b75 to 39a95ae Compare April 3, 2025 08:47
@Helco Helco mentioned this pull request Aug 1, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Helco