Implement Staged Tile Loading Pipeline #779
Conversation
- Change _maxSimultaneousRequests to 28 for testing - Put loadProgress calculation into ViewUpdateResult instead of being determined on the fly - Put loadProgress kick hack back in for testing
Add assertion to view results Rename some members
This was just a test. In practice, the view can change frequently. We want to drop the work that doesn't make it.
No reason to bump this. Latest tests show minimal improvement
(still much to do here)
|
Finished performance testing and updated the missing numbers. A snippet A little odd that Denver slowed down a bit. It is our smallest test, and already pretty fast. First guess is that this PR is indeed batching content requests better as shown for the google levels. But for cached (or very fast) content requests, it may be introducing delay (or more overhead) for processing. Like @kring said, this could be an errant I think this is ready for review and to talk a bit more about performance numbers. Making this a non-draft PR... |
|
I think I have a solid lead for the slowdown in the Google warm cache case... If I turn off the time budget in This function is throttling the final stage of tile loading based on a time budget, to keep a reasonable frame rate. In the context of this PR, we can think of it as the final 'stage' in our staged loading pipeline. And taking into account what this PR is doing, it seems likely that it's pushing the same amount of work through this stage, but with bigger batches of work, later in time, ultimately leading to longer load times. Will think on this a bit more... |
…processLoadRequests) Previously this was only happening in dispatchMainThreadTasks, at the beginning of update_view
- Handle completed work for newly dispatched work that completes immediately (like unit tests) - Add done loading notify for tiles that fail too (but don't count towards used bytes
|
Found the slowdown in the Google warm cache case! (finally). TLDR, the reference branch (based on main) I'm comparing to had a bug where it completed too early. Fixing the bug and doing a quick retest showed me that this PR doesn't seem to introduce any weird slow downs in this case anymore. Will retest to verify. I've fixed it in my branch here, but it could optionally be its own PR as well, since it's a bug in main too. More in depth, the problem is in Unfortunately, This behavior coincides with what I saw in the original tests. Inexplicably fast performance compared to my PR, where I had inadvertently fixed this bug through refactoring. It also coincides with my observation of equal performance when turning off time budget. Doing this clears the main thread queue every frame, which essentially makes it always 0 when |
|
Another update to the performance test numbers... Good news: the performance wins from this PR went up! (now 25-32% improvements for the google tests) Mediocre news: although the inexplicable "warm cache is really slow" problem is gone, warm cache results are still slightly slower in this PR (9-21% slower). Denver cold cache falls into this category too. I have a pretty good reason to believe that the problem is here, which coincides with @kring 's recommendation to look out for main thread continuations. The fix would be to dispatch processing work immediately, rather than waiting to dispatch on the main thread. Not a trivial change, but not an unreasonable amount of work either. A snippet |
|
I'm seeing frequent crashes just by press Play and flying around in the Here's one example: The immediate problem here appears to be that in
Here's another example: Here we're in
|
| // A response code of 0 is not a valid HTTP code | ||
| // and probably indicates a non-network error. |
There was a problem hiding this comment.
No, it probably indicates a valid response from a file:/// URL. It's annoying that libcurl returns a status code of 0 for a successful file read, but it does.
Those crashes look pretty severe and not obvious. I can't seem to reproduce them though. I checked several of the sample levels in PIE mode and everything seems to run fine. Ran in these 4 configurations...
(this PR with cesium-unreal main) Could there be something wrong with your set up? |
I don't have any reason to think so @csciguy8. But I'll try main to make sure it doesn't happen there. They look like race conditions to me, and I was able to reproduce various crashes in both Debug Editor (with Debug build of cesium-native) and Development Editor (with RelWithDebInfo build of cesium-native). |
|
I switched to main, flew around a whole bunch, and never saw a crash. Switched back to this branch and saw it within seconds. I can easily make it happen at will. I recorded a video, which I'll send to you separately. |
…een released When unloading tiles, make sure raster mapped tiles aren't loading, if so, wait for them to finish. This case can happen when moving very quickly through the world, where a tile is unloaded before it is finished loading.
In LayerJsonTerrainLoader::getLoadWork, when no quadtree tile ID is detected, don't skip adding all work, just the url request work.
@kring Ready for another look regarding those crashes. Thanks for the test. Looks like I never tested in cases where tiles would get to the unloading state faster than they would load. |
When loading tiles, implement a new staged pipeline where network requests are dispatched separately from processing work.
This achieves a 25-32% loading time reduction in some test cases.
Closes #746. Also closes #473 as a side effect.
Performance Tests Summary (Cesium for Unreal Performance Tests)
Complete Testing Data
In Depth
...
Simplified view of existing tile loads
This PR introduces this structure
The best place to start looking at the code changes would be in
Tileset:: _processWorkerThreadLoadQueue.Previous load requests were handled immediately, but now they are now passed to
TilesetContentManager, which transforms the requests into new data thatTileWorkManagercan work with, a generalizedTileLoadWork.TileWorkManagerhandles the critical logic to queue / throttle / and executeTileLoadWorkobjects.TO DO