Skip to content

Commit 0c669e5

Browse files
authored
Parallelize and simplify the asset processing loop. (#21701)
# Objective - The previous asset processing loop was very difficult to understand (IMO). - The initial processing of tasks would start a bunch of tasks. Then listening would listen for events and then await on processing tasks one at a time before continuing to listen to events. Finishing a task would also add paths to a **separate** `check_reprocess_queue` which would only be checked after all the current events have been handled. - Also processing tasks did not occur in parallel - so we'd process assets one at a time. ## Solution Approximately throw everything out. The asset processor now does these things: 1. Initialize the processor: same as before, recover from the transaction log, initialize the state of all processed assets (so we can lock them). 2. Queue all the initial processing tasks: iterate through all processed sources, finding all their assets, and queue a task for them (to recheck whether they need to be processed, and reprocess them if so). Note we don't spawn any bevy_task::Tasks here. 3. Spawn the "executor" bevy_task::Task: This task spawns the queued tasks and updates the overall state of processing (i.e., processing vs finished). 4. Spawn the source change event listeners: spawns a bevy_task::Task for each asset source to listen on its event receiver and queued up any new tasks as source assets change. So this parallelizes event processing from asset sources, parallelizes processing each asset, and (IMO) makes the whole processing loop much simpler. Also I think it's funny that parallelizing could make things simpler lol. ### Caveats - I've removed the public methods for `process_assets` and `listen_for_source_change_events`. My guess is these were public so that users can call them outside the context of a running app? I'm not entirely sure. I think this needs to be rethought though if that's the case. For one, a running app currently will not be gated on processing from another app, meaning things will probably get out-of-sync very easily. If need be, I think we can bring this back fairly straight forward. There also isn't a migration guide since there's nothing to migrate to here. - Parallelizing asset processing **could** be bad for very large tasks. Some GLTF files can get REALLY big, and managing memory there is very important (though we're still bad at this). So parallelizing asset processing can result in many tasks running concurrently consuming more memory without a way to control it. However I think this is a more general problem and we should find other solutions than "don't parallelize". ## Testing - The asset processing tests still pass! - The asset_processing example seems to behave the same!
1 parent 0bac137 commit 0c669e5

File tree

2 files changed

+222
-140
lines changed

2 files changed

+222
-140
lines changed

crates/bevy_asset/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ downcast-rs = { version = "2", default-features = false }
5353
disqualified = { version = "1.0", default-features = false }
5454
either = { version = "1.13", default-features = false }
5555
futures-util = { version = "0.3", default-features = false, features = [
56+
"async-await-macro",
5657
"alloc",
5758
] }
5859
futures-io = { version = "0.3", default-features = false }

0 commit comments

Comments
 (0)