echuraev commented on a change in pull request #7714:
URL: https://github.com/apache/tvm/pull/7714#discussion_r598418659



##########
File path: src/runtime/metal/metal_device_api.mm
##########
@@ -147,102 +155,110 @@ int GetWarpSize(id<MTLDevice> dev) {
 
 void* MetalWorkspace::AllocDataSpace(TVMContext ctx, size_t nbytes, size_t 
alignment,
                                      DLDataType type_hint) {
-  this->Init();
-  id<MTLDevice> dev = GetDevice(ctx);
-  // GPU memory only
-  MTLResourceOptions storage_mode = MTLResourceStorageModePrivate;
-  /*
-  #if TARGET_OS_IPHONE
-  storage_mode = MTLResourceStorageModeShared;
-  #else
-  storage_mode = MTLResourceStorageModeManaged;
-  #endif
-  */
-  id<MTLBuffer> buf = [dev newBufferWithLength:nbytes options:storage_mode];
-  ICHECK(buf != nil);
-  return (void*)(CFBridgingRetain(buf));
+  @autoreleasepool {
+    this->Init();
+    id<MTLDevice> dev = GetDevice(ctx);
+    // GPU memory only
+    MTLResourceOptions storage_mode = MTLResourceStorageModePrivate;
+    /*
+    #if TARGET_OS_IPHONE
+    storage_mode = MTLResourceStorageModeShared;
+    #else
+    storage_mode = MTLResourceStorageModeManaged;
+    #endif
+    */
+    id<MTLBuffer> buf = [dev newBufferWithLength:nbytes options:storage_mode];
+    ICHECK(buf != nil);
+    return (void*)(buf);

Review comment:
       `buf` won't be auto-released in this function. Quote from Apple 
documentation:
   > You take ownership of an object if you create it using a method whose name 
begins with “alloc” or “new” or contains “copy” (for example, alloc, newObject, 
or mutableCopy), or if you send it a retain message. You are responsible for 
relinquishing ownership of objects you own using release or autorelease. Any 
other time you receive an object, you must not release it.
   
   We create `buf` by using method with "new". So, the `newBufferWithLength` 
method returns the raw pointer on the memory not autoreleased pointer. Thus, we 
should call `release` when it will be necessary to free this memory.
   Also, I double-checked this by printing pointer address in `AllocDataSpace` 
and `FreeDataSpace` the addresses were the same.




-- 
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.

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


Reply via email to