try fix tgs agitate #751

zhaoguochun1995 · 2024-03-28T10:56:42Z

llama2 70B 1024卡, 10小时平均tgs从320提升至351
llama2 70B 1024卡,cpu利用率从11%降低至3%
llama2 70B 1024卡设备利用率波动减小，10小时内只有7次出现了利用率降低到30%的情况，其他时间能稳定维持在50%左右
https://aicarrier.feishu.cn/wiki/Eh4lwUgHkivsjVkPtupcDPptnYd

…ate2

…syncResourcePoolLength

lljbash · 2024-04-08T03:09:47Z

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUCachingAllocator.cpp

@@ -18,6 +18,15 @@ namespace dipu {
 // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 std::mutex DIPURawDeviceAllocator::mutex_;

+size_t kMaxAsyncResourcePoolLength = [](){


感觉得有个统一的入口控制一下，现在读取环境变量太分散了，不方便维护

嗯嗯，确实

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUCachingAllocator.cpp

wiryls · 2024-04-09T05:41:44Z

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUAsyncResourcePool.h

@@ -31,7 +29,11 @@ class AsyncResourcePoolImpl : public AsyncResourcePool<T> {
 public:
  void add(const T& t, std::deque<DIPUEvent>& events) override {
    std::lock_guard<mutex_t> lk(mutex_);
-    list_.emplace_back(t, std::move(events));
+    if (events.size() > 0) {


这里 events 为空需要加入 list 吗，我以为可以忽略它

需要的，不然会内存泄漏

建议后续把这块逻辑整体改下，对于没有在流上等待的 tensor，析构时直接 restore()。

这里实际上是故意没有在析构的时候restore。主要目的： 1. 加快tensor析构的速度 2. tensor析构时restore没有什么用，只有在申请的时候才需要尽可能多的内存已经回收。 3. 析构时里面回收，有可能流上还没有读写完毕，减小竞争的概率 4. resotre时可能会有碎片整理等操作，把潜在的耗时放在申请的时候，可以让一部分wait变成有意义的cpu操作

jfxu-st · 2024-04-09T07:39:09Z

dipu/torch_dipu/csrc_dipu/utils/env.hpp

+namespace dipu {
+
+template <typename T>
+T get_env_or_default(const char* env_name, const T& defalut_value) {


typo

Suggested change

T get_env_or_default(const char* env_name, const T& defalut_value) {

T get_env_or_default(const char* env_name, const T& default_value) {

jfxu-st · 2024-04-09T07:39:26Z

dipu/torch_dipu/csrc_dipu/utils/env.hpp

+T get_env_or_default(const char* env_name, const T& defalut_value) {
+  const char* env = std::getenv(env_name);
+  if (env == nullptr) {
+    return defalut_value;


ditto

Suggested change

return defalut_value;

return default_value;

jfxu-st · 2024-04-09T08:58:38Z

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp

-        std::this_thread::yield();
-        continue;
+        auto now = std::chrono::steady_clock::now();
+        auto elasped = now - start;


typo

Suggested change

auto elasped = now - start;

auto elapsed = now - start;

jfxu-st · 2024-04-09T08:59:10Z

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp

-        continue;
+        auto now = std::chrono::steady_clock::now();
+        auto elasped = now - start;
+        if (elasped < maxWaitTime) {


ditto

Suggested change

if (elasped < maxWaitTime) {

if (elapsed < maxWaitTime) {

dipu/torch_dipu/csrc_dipu/utils/env.hpp

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp

mrdanielw · 2024-04-10T02:41:18Z

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp

+          std::this_thread::yield();
+          continue;
+        }
+        return;


这个情况相当于如果等了一段时间还没ready 就不做empty_resource_pool了？如果是的话这个函数得设计下比如try empty 之类的如果最后没释放成功return个false 释放成功return个true 不然看到吗以为empty了就是释放掉了其实没做成？

嗯嗯，确实可以这样

这样内存不够的时候也会有问题了，得完善下

备注2个点：
根据刚才和国春的讨论，我备注2个点（都是个老问题，后续单独改即可，无需计入这个pr）。。
@yangbofun @mrdanielw @caikun-pjlab @Wrench-Git @jfxu-st

现有的 allocator 对于非默认流上 tensor 的分配有bug，无法保证内存安全，国春昨天提到一个简单的改法（所有分配都先 record stream），但是感觉对现有性能影响有点大。还有一种方法是 “分配时让非默认流先强制同步到默认流”，对现有代码影响小些，但对非默认流上的分配也不友好。

目前的 record Stream 调用有些地方（Copy, DIPUGuardImpl）是和默认流比较并选择忽略记录，这个在配合非默认流分配的tensor使用时也会出问题（非默认流上分配，然后转移到默认流上计算的tensor的记录会被忽略)。建议上层取消判断，统一由 allocator 来判断是否真的要记录（allocator 需要先能正确记录tensor 分配时的当前流，才能高效的完成这件事，否则默认流上copy 的Tensor 会退化为全部record了，把@jfxu-st 之前的优化又冲掉了）。

* add rms norm op * take functions_ext into the adaptor

try fix tgs agitate

2941910

zhaoguochun1995 requested review from mrdanielw, fandaoyi and lljbash as code owners March 28, 2024 10:56

zhaoguochun1995 and others added 10 commits March 28, 2024 13:17

async_mempool.length-0

d5f3696

use new param

a18bfd0

Merge remote-tracking branch 'origin/main' into zgc/dipu_fix_tgs_agit…

5a177b0

…ate2

reduce max extend size

9156868

kMaxAsyncResourcePoolLength->8,kMaxExtendSize->256MB

6139204

supports using the value of environment variables to initialize kMaxA…

cf57466

…syncResourcePoolLength

kMaxExtendSize->32MB

949f83d

kMaxExtendSize->128MB

f87d57e

enable init kMaxExtendSize using env var

ff5d314

enable init kMaxExtendSize using env var

2f48b62

lljbash reviewed Apr 8, 2024

View reviewed changes

zhaoguochun1995 added 2 commits April 8, 2024 07:11

fix lint

549e274

modify default value

ea2db53

zhaoguochun1995 force-pushed the zgc/dipu_fix_tgs_agitate2 branch from ff63f12 to ea2db53 Compare April 8, 2024 08:39

zhaoguochun1995 added 3 commits April 8, 2024 15:08

back up

e49f4cf

modify maxExtendSize 512->1024

c36dadb

fix lint

4128475

wiryls requested changes Apr 9, 2024

View reviewed changes

add get_env_or_default func

a580fbf

jfxu-st reviewed Apr 9, 2024

View reviewed changes

fix no need change

98dd050

zhaoguochun1995 force-pushed the zgc/dipu_fix_tgs_agitate2 branch from 33bcd11 to 98dd050 Compare April 9, 2024 08:12

fix typo error

3122100

jfxu-st reviewed Apr 9, 2024

View reviewed changes

wiryls reviewed Apr 9, 2024

View reviewed changes

dipu/torch_dipu/csrc_dipu/utils/env.hpp Show resolved Hide resolved

wiryls reviewed Apr 9, 2024

View reviewed changes

dipu/torch_dipu/csrc_dipu/runtime/core/allocator/DIPUBFCachingAllocator.cpp Outdated Show resolved Hide resolved

fix lint

f277255

zhaoguochun1995 force-pushed the zgc/dipu_fix_tgs_agitate2 branch 3 times, most recently from 13d7299 to 4029d2d Compare April 9, 2024 12:37

minor change

3a04678

mrdanielw reviewed Apr 10, 2024

View reviewed changes

zhaoguochun1995 force-pushed the zgc/dipu_fix_tgs_agitate2 branch from 4029d2d to 3a04678 Compare April 10, 2024 02:50

optimize BF cached-allocator logic

a5edb80

zhaoguochun1995 force-pushed the zgc/dipu_fix_tgs_agitate2 branch from 53c7755 to a5edb80 Compare April 10, 2024 02:58

yangbofun approved these changes Apr 10, 2024

View reviewed changes

minor change

cd605ff

wiryls approved these changes Apr 10, 2024

View reviewed changes

mrdanielw merged commit f01a8c1 into DeepLink-org:main Apr 10, 2024
30 checks passed

wiryls deleted the zgc/dipu_fix_tgs_agitate2 branch April 11, 2024 10:00

xuq7410 pushed a commit to xuq7410/deeplink.framework that referenced this pull request May 23, 2024

Zzf/rms norm (DeepLink-org#751)

d50f920

* add rms norm op * take functions_ext into the adaptor

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

try fix tgs agitate #751

try fix tgs agitate #751

zhaoguochun1995 commented Mar 28, 2024 •

edited

Loading

lljbash Apr 8, 2024

wiryls Apr 8, 2024 •

edited

Loading

zhaoguochun1995 Apr 10, 2024

wiryls Apr 9, 2024

zhaoguochun1995 Apr 9, 2024

fandaoyi Apr 9, 2024

zhaoguochun1995 Apr 10, 2024

jfxu-st Apr 9, 2024

jfxu-st Apr 9, 2024 •

edited

Loading

jfxu-st Apr 9, 2024

jfxu-st Apr 9, 2024

mrdanielw Apr 10, 2024

zhaoguochun1995 Apr 10, 2024

zhaoguochun1995 Apr 10, 2024

fandaoyi Apr 10, 2024 •

edited by jfxu-st

Loading

	T get_env_or_default(const char* env_name, const T& defalut_value) {
	T get_env_or_default(const char* env_name, const T& default_value) {

try fix tgs agitate #751

try fix tgs agitate #751

Conversation

zhaoguochun1995 commented Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

wiryls Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfxu-st Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fandaoyi Apr 10, 2024 • edited by jfxu-st Loading

Choose a reason for hiding this comment

zhaoguochun1995 commented Mar 28, 2024 •

edited

Loading

wiryls Apr 8, 2024 •

edited

Loading

jfxu-st Apr 9, 2024 •

edited

Loading

fandaoyi Apr 10, 2024 •

edited by jfxu-st

Loading