You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/08/05 16:19:35 UTC

[GitHub] [tvm] shtinsa opened a new pull request, #12323: The change allows to increase performance in multi-thread environment.

shtinsa opened a new pull request, #12323:
URL: https://github.com/apache/tvm/pull/12323

   In this case data locality is improved and it may have positive effect to final inference in case of MT execution.
   
   Thanks for contributing to TVM!   Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
   


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


[GitHub] [tvm] apeskov commented on a diff in pull request #12323: The change allows to increase performance in multi-thread environment.

Posted by GitBox <gi...@apache.org>.
apeskov commented on code in PR #12323:
URL: https://github.com/apache/tvm/pull/12323#discussion_r952377650


##########
src/runtime/vm/memory_manager.cc:
##########
@@ -105,15 +105,16 @@ NDArray StorageObj::AllocNDArray(size_t offset, std::vector<int64_t> shape, DLDa
   return ret;
 }
 
-MemoryManager* MemoryManager::Global() {
+std::shared_ptr<MemoryManager> MemoryManager::Global() {

Review Comment:
   "Global" means global. Thread local object is not obvious behaviour for this method.  



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


[GitHub] [tvm] shtinsa commented on pull request #12323: The change allows to increase performance in multi-thread environment.

Posted by GitBox <gi...@apache.org>.
shtinsa commented on PR #12323:
URL: https://github.com/apache/tvm/pull/12323#issuecomment-1206755038

   @jwfromm @tkonolige could you please review? the changes may increase memory usage but the same time it decrease data crashing within the cpu caches. 


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


[GitHub] [tvm] shtinsa commented on pull request #12323: The change allows to increase performance in multi-thread environment.

Posted by GitBox <gi...@apache.org>.
shtinsa commented on PR #12323:
URL: https://github.com/apache/tvm/pull/12323#issuecomment-1206740036

   ![res_dlrm-0805_three](https://user-images.githubusercontent.com/88086617/183139547-0f2bca2a-c535-4851-a9c5-230662ac591f.png)
   The picture contains perf improvements in case of DLRM model
   


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


[GitHub] [tvm] apeskov commented on pull request #12323: The change allows to increase performance in multi-thread environment.

Posted by GitBox <gi...@apache.org>.
apeskov commented on PR #12323:
URL: https://github.com/apache/tvm/pull/12323#issuecomment-1223805016

   The word "Global" means that it's a global singleton. Changing semantic to thread local will confuse developers. 
   
   Moreover, this patch works only because you initialise VirtualMachine modules form separate threads. That is not mandatory behaviour. Customer may create a set of VirtualMachine in main thread and after that assign them to worker sub threads.
   
   This change is good enough to demonstrate possibility of advance memory management in case of multi instance execution. But it cannot be merged as is.


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