Issue #50: Make execution_resource iterable. #73
Conversation
* Replace execution_resource::resources with execution_resource::begin and execution_resource::end. * Add execution_resource::size. * Add execution_resource::operator[]. * Add aliases to execution_resource to support new interface. * Improve woring for execution resource and system topology. * Update examples to suppoort change. * Rename this_system::resources to this_system::discover_topology.
| *Returns:* An `execution_resource` object exposing the **system execution resource**. | ||
|
|
||
| *Returns:* An `std::vector` containing all *system level resources*. | ||
| *Requires:* If `this_system::discover_topology().size() > 0`, `this_system::discover_topology()[0]` be the `execution_resource` use by `std::thread`. Calls to `this_system::discover_topology()` may not introduce a data race with any other call to `this_system::discover_topology()`. |
There was a problem hiding this comment.
Does that mean the implementation is responsible to implementing mutex or similar to ensure no data races?
This means the call to discover topology is (even more) expensive. Seems to me, since this is a function that will be called by the library programmer or the programmer on controlled situations, its better for them to handle the control of accessing this function.
There was a problem hiding this comment.
Yes, the current wording requires that discover_topology remain race free with respect to other calls to discover_topology. This prevents two simultaneous calls to discover_topology from interfering with each other and potentially trying to initialise or finalise the same third-party APIs at the same time. We felt this a fairly low burden to users as topology discovery will generally happen away from the critical path and this will only incur a cost if users were to have multiple threads potentially capable of performing topology discovery. And it means users can safely do this if they want.
This doesn't cover the case where users may manually call a third-party API which invokes the same APIs as discover_topology, however, we felt this is not something we can protect users from, and implementations should accurately document any dependencies they have.
This also doesn't currently cover is any potential races between using a resource being used (for example in some execution context) whilst performing topology discovery. This is a slightly trickier case to resolve which we will have to look at.
| *Effects:* Discovers all **execution resources** available within the system and constructs the **system topology** DAG, describing a read-only snapshot at the point of the call. | ||
|
|
||
| > [*Note:* Returning a `std::vector` allows users to potentially manipulate the container of `execution_resource`s after it is returned. We may want to replace this at a later date with an alternative type which is more restrictive, such as a range or span. *--end note*] | ||
| *Throws:* Any exception thrown as a result of **system topology** discovery. |
There was a problem hiding this comment.
So if this throws, does that mean the system topology is unusable? what can the user do to handle that? Either terminate or assume a sequential execution. Probably worth adding some questions for the committee as to what to do when the discovery fails, and what users would expect. Since the discover topology can call different libraries at different points in time, it may be that the first time is called works, but the second doesnt (if there is another initialize_library in between). Even more interesting is the opposite, a call that fails, then a user disables the library, and then discover_topology works again. This should be described or clarified at some point (again, possibly not in this proposal!)
There was a problem hiding this comment.
Yes at the moment this is the case. This is something which we've discussed before, but I think we should spend some more time discussing. I responded on this in more detail in a comment above and created an issue for discussing it further (see #79).
* Resolve conflicts. * Resolve change list.
* Add minor corrections. * Add requirement for execution_resource iterators to be random access. * Add section to background discussing handle partial errors on topology discovery.
and execution_resource::end.