[GitHub] [tvm] adstraw commented on a diff in pull request #12727: [Hexagon] [runtime] Improve runtime resource management

[GitBox <gi...@apache.org>]

adstraw commented on code in PR #12727:
URL: https://github.com/apache/tvm/pull/12727#discussion_r968868937


##########
src/runtime/hexagon/hexagon_device_api.h:
##########
@@ -45,11 +45,27 @@ class HexagonDeviceAPI final : public DeviceAPI {
   static HexagonDeviceAPI* Global();
 
   //! \brief Constructor
-  HexagonDeviceAPI() {}
+  HexagonDeviceAPI() { hexbuffs = std::make_unique<HexagonBufferManager>(); }
 
   //! \brief Destructor
   ~HexagonDeviceAPI() {}
 
+  //! \brief Creates resource managers for the runtime
+  void AcquireResources() {
+    if (!hexbuffs->empty()) {
+      LOG(INFO) << "hexbuffs was not empty in AcquireResources";

Review Comment:
   Would it make more sense to put `hexbuffs = std::make_unique<HexagonBufferManager>();` in this function (`AcquireResources`) rather than in the constructor for the device API.  This way, we know that `hexbuffs` is empty at the time we run "acquire" - no need to check.  It would also alleviate the need to check whether `hexbuffs` is empty on "release" and we could simply `hexbuffs.reset()`.
   
   ```
   void AcquireResources() {
     CHECK_EQ(hexbuffs, nullptr);
     hexbuffs = std::make_unique<HexagonBufferManager>();
   }
   
   void ReleaseResources() {
     hexbuffs.reset();
     hexbuffs = nullptr;
   }
   ```
   
   The device api ctor could just set hexbuffs=nullptr.



-- 
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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org