Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Affinity: Update resource lifetime #70
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
base: master
Are you sure you want to change the base?
Affinity: Update resource lifetime #70
Changes from all commits
ace72af
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I understand the part about the thing that is pointed to being dynamic. Does this mean that the thing that an
execution_resource
points to may become available/unavailable dynamically, not that it can become something different?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for your feedback! I'll answer a few questions at a time since I'm in meetings all day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean by "the thing that is pointed to," is that an execution resource may point either to a hardware resource (CPU core, etc.) or to a software resource. Tom Rodgers from Red Hat pointed out today the use case of running in a virtual machine, in which "hardware" doesn't necessarily map to real hardware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case of running in a virtual machine, the "CPU core" to which an execution resource points, might run on one physical CPU core at one moment. At the next, it may run on a different CPU core, even possibly on a different node in a different place. Nevertheless, we want the execution resource to point to that same "virtualized CPU core"; the resource shouldn't suddenly point to something else like a GPU or whatever.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your explanation, I think I understand now, so effectively we want to say that a C++ library implementation cannot change what an execution resource points to dynamically, however, what that execution resource points to can be dynamically managed by the environment which is executing the process, such as the OS or a virtual machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right; thanks! :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Note to self: Added virtual machine example locally)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've raised some interesting questions here.
I think the best way to solve this would be to require that
this_system::get_resource
be thread-safe and initialise and finalize any third party or OS resources when constructing the topology before returning. This would mean that there's no way to concurrently interfere with how the implementation uses those resources without using them directly through their respective API, though that would have to be undefined behaviour. This also means that the topology would be a snapshot of the resources at that point, and don't actually require any of the underlying resources to be alive until they are actually used, i.e. when passed to the construction of a context.The disadvantage of this approach is that the initial call to
this_system::get_resource
could be quite costly, depending on the implementation, though I'd expect that you would only do this once on startup or at specific intervals between computations.Alternatively, if we go down the route that was suggested at Rapperswil of having a visitor approach to topology discovery, where the user provides some kind of visitor that traverses the topology and returns the resources that it wants to use. With this kind of approach, it might be worth having the
execution_resource
have active resources. Perhaps this pattern could construct the context directly?It may also be worth having a thread-safe
valid
member function on theexecution_resource
to check if the underlying resource is still available. Though I think this would have to be named differently to represent that it is not simply a getter and that it must do some querying or initialising of resources within the topology to identify if the resource is still valid. However, even with this, you could still have the situation where a resource becomes unavailable between callingvalid
and constructing an execution context or after the context is constructed.Perhaps instead we should say that you can always construct a context from an
execution_resource
, even if it's unavailable, but have the context handle the case where the resource is unavailable, by cancelling enqueued work and disallowing new submissions and throwing an exception or calling a callback function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. This could even solve the problem of some resources only being available inside a parallel region (like a GPU). Topology is a snapshot of everything reachable from the root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with that being UB in general, possibly with specific APIs giving stronger guarantees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right -- this is really about constructing a thread pool. Crazy high-frequency dynamic hardware load balancing feels like it wants a different interface. (Users might just see that as a special kind of resource -- it doesn't feel like something that naturally has an easily walkable topology.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was that in the notes? I'd like to learn more about that. (Sorry I missed that discussion at Rapperswil.) I'm not sure, without knowing more at least, whether that would solve the problems we've discussed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just attempt to create the context? If the resource is no longer available, the context creation fails. Otherwise, the context assumes the responsibility for keeping the resource alive or otherwise handling the case where the resource ceases to be alive at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AerialMantis I reviewed the Rapperswil notes, and noticed that the affinity paper was marked "DND." It looks like it wasn't covered there. Were you referring to the P0443 discussion in SG1 (and/or joint with LEWG)? Here is the link (requires usual login). For example: "JB: My takeaway is that (P0443) executors are copyable things with reference semantics."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Discussion a week or two ago concluded that the affinity paper was actually discussed in SG1 in Rapperswil, but didn't make it into the minutes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Note to self: I think I've addressed this in my local changes.)