Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support async copy for TensorFromVector with event #32563

Merged
merged 1 commit into from
May 12, 2021
Merged

Support async copy for TensorFromVector with event #32563

merged 1 commit into from
May 12, 2021

Conversation

liym27
Copy link
Contributor

@liym27 liym27 commented Apr 26, 2021 •
edited
Loading

PR types

Performance optimization

PR changes

Others

Describe

Support async copy for TensorFromVector and FillNpuTensorWithConstant with event.

1. 功能：

利用event 机制，在TensorFromVector 和FillNpuTensorWithConstant中支持异步数据拷贝。

2. 收益：

2.1 正确性：保证异步拷贝数据，在拷贝完成前不会被析构

  • 问题：cpu 数据 异步拷贝到 npu Tensor 时，在拷贝完成前，cpu 数据被提前析构，导致拷贝结果不正确。
  • 解决方案：
    • event 机制保证顺序->"先拷贝再析构"：
      在cpu->npu 拷贝操作后，插入 event 到 stream，数据析构时，查询该 event 是否完成，未完成则延迟析构数据，保证"拷贝完 再析构"。
    • NPUPinnedAllocator 管理内存，达到”延迟析构“的目的：
      先将 cpu 数据 拷贝到一个 cpu tensor，aka npu pinned tensor，然后将 npu pinned tensor 拷贝到 npu tensor。通过 NPUPinnedAllocator 管理 npu pinned tensor 的内存分配和释放。NPUPinnedAllocator 在 npu pinned tensor 对应的 event 完成后，再释放内存。该功能在 [NPU] Support npu pinned allocator #32840 中实现

2.2 性能提升：

异步拷贝 比 同步拷贝，有性能提升。单机训练 ernie3.0，性能如下表所示：

序号 方案 速度（tokens/s） 性能提升
1 FillNpuTensorWithConstant 19988.26 base
2 async 20545.58 +2.79%
3 async + malloc/free 21706.29 +8.60%
4 async + malloc/free + 避免shape为[1]时cpu tensor拷贝 21759.33 +8.70%
5 async + malloc/free + 避免shape为[1]时cpu tensor拷贝+thread 21466.57 +7.40%

本PR代码是方案4。

方案说明：

  • FillNpuTensorWithConstant：调用FillNpuTensorWithConstant，实现常量Tensor
  • async：通过event 机制，异步拷贝数据，保证数据正确性
  • malloc/free：不用昇腾接口 aclrtMallocHost/aclrtFreeHost，使用c++ malloc/free
  • 避免shape为[1]时cpu tensor拷贝：shape为[1]的变量的拷贝，直接修改tensor 数据，不调用Copy函数
  • thread：线程批量析构数据

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot-old
Copy link

paddle-bot-old bot commented Apr 26, 2021 •
edited
Loading

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from fe750df to e23c489 Compare April 26, 2021 07:56
zhiqiu
zhiqiu reviewed Apr 27, 2021
View reviewed changes
paddle/fluid/memory/allocation/allocator.h Outdated
Comment on lines 124 to 127
VLOG(4) << " --------- liym27: TopDecoratedAllocator : "
" decorated_allocators_.size() = "
<< decorated_allocators_.size();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug code can be reduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch 2 times, most recently from 9955b3f to 854a11d Compare April 28, 2021 12:37
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 854a11d to dff54a0 Compare April 28, 2021 23:14
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 30da90c to 09c562a Compare April 29, 2021 08:19
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 09c562a to 8bfd2f8 Compare April 29, 2021 11:15
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 8bfd2f8 to 52c2ced Compare April 29, 2021 11:45
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 52c2ced to 1fd121a Compare April 29, 2021 12:37
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 1fd121a to 67a5fb8 Compare April 29, 2021 13:02
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 67a5fb8 to 8e5a23b Compare April 30, 2021 00:05
zhiqiu
zhiqiu previously approved these changes Apr 30, 2021
View reviewed changes
Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

paddle/fluid/memory/detail/system_allocator.cc Outdated

void* p;
// PINNED memory is visible to all NPU contexts.
auto result = aclrtMallocHost(&p, size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be replaced with malloc in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will fix it next PR

@liym27 liym27 requested a review from phlrain April 30, 2021 07:52
zhhsplendid
zhhsplendid previously approved these changes Apr 30, 2021
View reviewed changes
Copy link
Member

@zhhsplendid zhhsplendid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paddle-bot-old
Copy link

paddle-bot-old bot commented May 8, 2021

Sorry to inform you that 8e5a23b's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@liym27 liym27 closed this May 10, 2021
@liym27 liym27 reopened this May 10, 2021
@liym27 liym27 dismissed stale reviews from zhhsplendid and zhiqiu via c1ba6df May 10, 2021 10:07
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 8e5a23b to c1ba6df Compare May 10, 2021 10:07
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from c1ba6df to 4dc032d Compare May 10, 2021 10:51
zhiqiu
zhiqiu previously approved these changes May 11, 2021
View reviewed changes
Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@liym27
Support async copy for TensorFromVector with event
2f6fc70 
Use malloc/free to replace aclrtMalloc/FreeHost

Avoid vector->npu_pinned_tensor when numel==1

Polish code

Fix compile error
@liym27 liym27 force-pushed the npu_asyc_copy_with_event branch from 4dc032d to 2f6fc70 Compare May 12, 2021 06:24
zhiqiu
zhiqiu approved these changes May 12, 2021
View reviewed changes
Copy link
Contributor

@zhiqiu zhiqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@liym27 liym27 merged commit 85512d6 into PaddlePaddle:develop May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhiqiu zhiqiu zhiqiu approved these changes

@zhhsplendid zhhsplendid zhhsplendid left review comments

@pangyoki pangyoki Awaiting requested review from pangyoki

@phlrain phlrain Awaiting requested review from phlrain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@liym27 @zhiqiu @zhhsplendid