Issue #41: Separate execution and memory resources #80
Conversation
* Fixes codeplaysoftware#41 and codeplaysoftware#42. * Introduce `memory_resource` to represent the memory component of a system topology. * Remove `can_place_memory` and `can_place_agents` from the `execution_resource` as these are no longer required. * Remove `memory_resource` and `allocator` from the `execution_context` as these no longer make sense. * Update the wording to describe how execution resources and memory resources are structured. * Refactor `affinity_query` to be between an `execution_resource` and a `memory_resource`. * Make minor corrections.
|
|
||
| auto relativeLatency = relativeLatency01 > relativeLatency02; | ||
| auto relativeLatency = relativeLatency1 > relativeLatency0; |
There was a problem hiding this comment.
What are the requirements for the returned type of the comparison operators? Must they be bool?
There was a problem hiding this comment.
Classes like less<T> break if the comparison operators don't return bool, because their operator() expects to return bool. This issue came up with bitmasks in the SIMD proposal; the author avoided the issue by not overloading comparison operators to return non-bool, and instead introducing two-letter comparison operators that return a bitmask for use e.g., in where clauses.
There was a problem hiding this comment.
See e.g., [comparisons.less]. I also read [expr.rel] as requiring that operator< always return bool.
There was a problem hiding this comment.
The return type of the comparison operators is currently a std::expected<size_t, error_type>, the reason for this was so that we could reflect a magnitude rather than simply a boolean as the relative affinity, and also that we want to handle failures to retrieve affinity. However, there have been some concerns about this interface not playing well with others, and this is another good example of that. I have an issue open for looking into alternative solutions to this - #68.
affinity/cpp-20/d0796r3.md
Outdated
|
|
||
| The construction of an `execution_context` on a component implies affinity (where possible) to the given resource. This guarantees that all executors created from that `execution_context` can access the resources and the internal data structures requires to guarantee the placement of the processor. | ||
| The construction of an `execution_context` on an `execution_resource` implies affinity (where possible) to the given resource. This guarantees that all executors created from that `execution_context` can access the resources and the internal data structures requires to guarantee the placement of the processor. |
There was a problem hiding this comment.
"data structures requires" -> "data structures required"
Not sure what "guarantee the placement of the processor" means. Has "processor" been defined in this context?
There was a problem hiding this comment.
"placement of the data" perhaps?
There was a problem hiding this comment.
Good catch, I believe this wording is describing the guarantee that executors can make about where execution agents run. I've changed this to "binding of execution agents", does this read better?
memory_resourceto represent the memory component of a system topology.can_place_memoryandcan_place_agentsfrom theexecution_resourceas these are no longer required.memory_resourceandallocatorfrom theexecution_contextas these no longer make sense.affinity_queryto be between anexecution_resourceand amemory_resource.