-
Notifications
You must be signed in to change notification settings - Fork 212
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.
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
Extend AsyncSystem with support for throttling groups, prioritization, and cancelation #789
Comments
This idea seems sound, and first reaction is "Why not?". If your code is arranged in such a way to take advantage of throttling groups, then go for it. But that "if" is really my only criticism. If this idea is built out, will it be useful? From what I've learned in this PR, building a stage loading pipeline, the bulk of the work was refactoring. Code needs to be structured in such a way that it can be throttled. The actual "throttling" part wasn't all that sophisticated. For example, this function throttles a pending queue of content requests to the Would it be worth refactoring that to use ThrottlingGroups? I'm not sure. Would it be useful for someone else? Maybe. |
@csciguy8 I'm way late getting back to you here, but I'm a little confused by your criticism.
The entire point of this proposal is that it avoids all that refactoring and allows the async system - which we're already using - to handle this all itself. So in the old implementation, the loading is done by
That's not exactly right, because loadTileContent initially expects it is called in the main thread. So actually we'd want to do that little bit of main-thread checking that it does first and only do the actual background work inside that thenInWorkerThread. But I don't think that detracts from the main point here. We would also need to pass the Compared to my (still early) understanding of #779, the main differences are:
The downside, though, is that this system is likely to be at least somewhat difficult to implement. |
No worries about any confusion. My last comment was 3 months ago, but I think we're getting sync'd up better now :) I'm glad you've looked at #779, and now that I've been working on other things for ~2 months now, I have some thoughts about it too. Overall, the end result works, and seems to produce better performance. The biggest issue I have is the general code direction and the extensive "refactoring" that I mentioned. Did my changes make sense with how we use our async system? Maybe they are even fighting it? Back to this issue and the proposal for throttling groups... I'm more inclined to be favor of a solution like this. I like the simplicity of defining the groups and throttling behavior. I like how it's extending and working with our existing paradigms. If all the complexities of throttling can be tucked away and no extensive code refactoring is necessary, excellent!
Can agree it would be difficult. And once the feature is implemented, how complex is it? Is it easy to verify correct functionality or maintain in the future? Hard to say without just building it first. |
Motivation
AsyncSystem::runInWorkerThread
(as the name implies) runs a task in a worker thread and returns a Future that resolves when it completes. If this work is CPU bound - as it usually is - it's not desirable to run too many such tasks simultaneously, because the overhead of task switching will become high and all of the tasks will complete slowly.In practice, though,
runInWorkerThread
is dispatched via some sort of thread pool. In the case of Cesium for Unreal, these tasks are dispatched to Unreal Engine's task graph, which is similar. The thread pool limits the number of tasks that run simultaneously; any extra tasks are added to a queue. As each task completes, the next task in the queue is dispatched.This scheme is strictly first in, first out (FIFO). Once
runInWorkerThread
(or similarly,thenInWorkerThread
) is called, the task will run eventually (process exit notwithstanding), and multiple tasks dispatched this way will start in the order in which these methods were called. There is no possibility of canceling or reprioritizing a task that hasn't started yet.On top of this, not all tasks are CPU bound. Cesium Native also needs to do HTTP requests. Network bandwidth, like CPU time, is a limited resource; attempting to do a very large number of network requests simultaneously is inefficient. While thread pools allow
AsyncSystem
to use CPU time efficiently, there is no similar mechanism for HTTP requests, GPU time, or any other type of limited resource.These are pretty big limitations when it comes to complicated asynchronous processes like loading 3D Tiles content. To load tile content, we need to:
We have ad-hoc ways of doing some approximation of this. Currently, there is a "number of simultaneous tile loads" per tileset. A tile that is doing any kind of loading - whether network or CPU - counts against this limit. This is inefficient in terms of both network and CPU utilization, as described in #473. We also can't cancel or reprioritize tile loads once they're started, as described in #564.
Proposal
This part is a work in progress! I don't think I have all the details right yet.
AsyncSystem
should make this sort of thing easy. First, we define a throttling group:We'll have a
ThrottlingGroup
instance for network requests, and another instance for CPU-bound background work.We also define a
TaskController
class that is used to cancel and prioritize an async "task", which is essentially a chain of Future continuations:The idea is that we can then write code like this:
AsyncSystem::withController
specializes the AsyncSystem for a given task. It allows the continuations created within it to be prioritized and canceled as a group.beginThrottle
returns aFuture
that resolves when the task should start. This may not happen right away if too many other tasks are already in progress within the throttling group. When the continuation chain reachesendThrottle
, the throttled portion of the task is complete and other tasks waiting in the same throttling group may begin (beginning with the one that is now highest priority).In this example, we do a network request. Then do throttled processing of the response in a worker thread. Depending on the result of some function call, we may need to do another network request, followed by more CPU work.
The overload of
IAssetAccessor::get
that takes aThrottlingGroup
looks like this:So the network requests happen in one throttling group, while the CPU processing happens in another. When a continuation chain reaches a
beginThrottle
, the task exits the current throttling group (if any), and enters the new one. When the continuation chain reaches theendThrottle
, the previous throttling group is re-entered.The text was updated successfully, but these errors were encountered: