jorisvandenbossche commented on code in PR #40699:
URL: https://github.com/apache/arrow/pull/40699#discussion_r1539762354


##########
cpp/src/arrow/device.h:
##########
@@ -363,4 +363,33 @@ class ARROW_EXPORT CPUMemoryManager : public MemoryManager 
{
 ARROW_EXPORT
 std::shared_ptr<MemoryManager> default_cpu_memory_manager();
 
+using MemoryMapper =
+    std::function<Result<std::shared_ptr<MemoryManager>>(int64_t device_id)>;

Review Comment:
   Good points, that naming is definitely more consistent. 
   
   There is however one problem that we already define a `DeviceMemoryMapper` 
for the keyword type in the actual bridge.h Import methods:
   
   
https://github.com/apache/arrow/blob/434f87274e8e9adab4f0434ae494f30dc955ca6e/cpp/src/arrow/c/bridge.h#L218-L219
   
   and we should probably find a distinct name, given that both are slight 
different (the one takes device_type+device_id and returns a MemoryManager, 
while the other is a function already tied to a specific device_type and thus 
only takes a device_id, returning again a MemoryManager)
   
   It's of course a subtle difference that might be difficult to embody in a 
name. But at least using distinct names seems best.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to