Remove almost all usage of `Blob` in EagerBlobObject #7895

daquexian · 2022-03-25T08:44:38Z

改动：

EagerBlobObject 不再是 BlobObject 的子类。删掉了 BlobObject 和另一个子类 LazyRefBlobObject，因为已经没有地方用到
user_op::Tensor 和 user_op::TensorDesc 的数据来源改成了 EagerBlobObject 自己的 shape/data_type/is_dynamic 成员而不再是 Blob
删掉了 TensorView 指令，因为已经不需要操作 Blob
受到现实难度的限制，blob_ 目前仍然是 EagerBlobObject 的一个成员（但不再需要手动维护了），完全去掉要等下个 PR
ShapeView/MutShapeView 不再以引用而是以值传递了，EagerBlobObject 的 shape_view() 方法会现场从 shape_ 构造出一个 ShapeView 对象。原因是
1. 引用传递要求 EagerBlobObject 同时维护 shape_、shape_view_ 和 mut_shape_view_ 三个对象，而且只有上层代码才知道什么时候需要同步这三个对象的内容。
2. 一般来讲 View 类型都是推荐用值传递的：https://stackoverflow.com/questions/27256377/c-view-types-pass-by-const-or-by-value
删除了 Shape 对象能在 O(1) 时间内得到 element count 的特性，这个特性阻碍了从 Shape 得到 MutShapeView，而且遍历一次 dim_vec 计算 element count 应该不会很耗时（实际情况还要跑 ci 看看）

Signed-off-by: daquexian <daquexian566@gmail.com>

oneflow/core/framework/nn_graph.h

oneflow/core/framework/tensor_methods.cpp

oneflow/core/job/runtime.cpp

oneflow/core/register/register_manager.cpp

Signed-off-by: daquexian <daquexian566@gmail.com>

github-actions · 2022-03-29T08:51:30Z

Code got formatted by CI. Please request CI again if you still want to have this PR merged. If the PR is from a forked repo, please download the patch files from the GitHub Actions web page and apply them locally.

oneflow/core/common/shape.h

Flowingsun007 · 2022-03-29T13:23:12Z

swin性能测试

环境

vs010
swin:ce80b517b9f8e53b39ba05c058048b133eebf698

数据

master分支@e62d826

200iter	显存(mb)	速度(s)
1n1g	5512*1	27
1n4g	5670*4	28
1n8g	5850*8	35

cpu 1n8g 1iter 145s

本分支@18215dc

200iter	显存(mb)	速度(s)
1n1g	5502*1	27
1n4g	5680*4	28
1n8g	5850*8	35

cpu 1n8g 1iter 142s

结论

基于swin测试发现：

1.cuda下，此分支和master上速度、显存占用在单卡-多卡下都相近，几乎没有差异；
2.猜测该分支改动带来的收益在cpu环境（view op）下较为明显，在cuda环境下几乎没有影响；后续需要根据德澎einsum的case：https://github.com/Oneflow-Inc/OneTeam/issues/1133#issue-1157969421 用nsight profile一下，才能准确看出优化程度。

Signed-off-by: daquexian <daquexian566@gmail.com>

Flowingsun007 · 2022-03-30T12:39:40Z

oneflow/core/kernel/blob_tensor_view.h

@@ -29,8 +29,8 @@ class BlobTensorView final : public Tensor {
  explicit BlobTensorView(Blob* blob);
  ~BlobTensorView() = default;

-  const ShapeView& shape() const override;
-  MutShapeView* mut_shape() override;
+  ShapeView shape() const override;


这块为啥去掉const

这个是返回一个值，所以不用加 const，以及 ShapeView 本身就没有写数据的操作（和 MutShapeView 对应）

Flowingsun007 · 2022-03-30T12:41:11Z

oneflow/core/memory/memory_allocator.cpp

+void InitNonPODTypeEagerBlobObjectIfNeed(MemoryAllocator* allocator,
+                                         vm::EagerBlobObject* eager_blob_object_ptr) {
+  if (eager_blob_object_ptr->data_type() == kOFRecord) {
+    int64_t elem_cnt = eager_blob_object_ptr->shape().elem_cnt();


int64_t elem_cnt = eager_blob_object_ptr->shape().elem_cnt();可以提取到
L124之前

github-actions · 2022-04-15T09:02:30Z

CI failed when running job: cuda-benchmark. PR label automerge has been removed

github-actions · 2022-04-16T14:42:32Z

Code got formatted by CI. Please request CI again if you still want to have this PR merged. If the PR is from a forked repo, please download the patch files from the GitHub Actions web page and apply them locally.

github-actions · 2022-04-19T08:14:56Z

Code got formatted by CI. Please request CI again if you still want to have this PR merged. If the PR is from a forked repo, please download the patch files from the GitHub Actions web page and apply them locally.

Signed-off-by: daquexian <daquexian566@gmail.com>

github-actions · 2022-04-28T13:14:54Z

Speed stats:

github-actions · 2022-04-29T08:21:00Z

CI failed when running job: Build cpu. PR label automerge has been removed

github-actions · 2022-04-29T22:28:06Z

CI failed when running job: cpu-misc. PR label automerge has been removed

github-actions · 2022-04-29T22:28:50Z

Speed stats:

Signed-off-by: daquexian <daquexian566@gmail.com>

github-actions · 2022-05-03T09:10:58Z

View latest API docs preview at: https://staging.oneflow.info/docs/Oneflow-Inc/oneflow/pr/7895/

lixinqi · 2022-05-03T12:48:51Z

后来有没有查到一开始性能受损的原因？

daquexian · 2022-05-03T14:25:08Z

后来有没有查到一开始性能受损的原因？

没有，可能是 speed test 退役、新版 benchmark 上线之后，就能通过了

daquexian added 2 commits March 25, 2022 15:56

remove blob from eager blob object

7ae55d7

Signed-off-by: daquexian <daquexian566@gmail.com>

add 'Blob()' in EagerBlobObject, remove BlobObject

15c31aa

Signed-off-by: daquexian <daquexian566@gmail.com>

daquexian added automerge system refactor labels Mar 25, 2022

daquexian requested review from lixinqi, chengtbf and strint as code owners March 25, 2022 08:44

daquexian added the enhancement label Mar 25, 2022

lixinqi reviewed Mar 25, 2022

View reviewed changes

daquexian added 3 commits March 29, 2022 14:15

Merge branch 'master' into remove_blob_from_eager_blob_object

03f9cc3

pass ShapeView by value, remove O(1) elem_cnt() feature of Shape

5078fee

Signed-off-by: daquexian <daquexian566@gmail.com>

address comments

6114b70

Signed-off-by: daquexian <daquexian566@gmail.com>

daquexian removed the automerge label Mar 29, 2022

daquexian requested a review from oneflow-ci-bot March 29, 2022 08:49

auto format by CI

18215dc

lixinqi approved these changes Mar 29, 2022

View reviewed changes

Flowingsun007 reviewed Mar 29, 2022

View reviewed changes

oneflow/core/common/shape.h Show resolved Hide resolved

daquexian and others added 2 commits March 30, 2022 10:54

remove unused variable

578c517

Signed-off-by: daquexian <daquexian566@gmail.com>

Merge branch 'master' into remove_blob_from_eager_blob_object

6861ae1

daquexian enabled auto-merge (squash) March 30, 2022 12:31

Flowingsun007 reviewed Mar 30, 2022

View reviewed changes

Flowingsun007 approved these changes Mar 30, 2022

View reviewed changes

daquexian added the automerge label Mar 30, 2022

mergify bot added 3 commits March 30, 2022 14:16

Merge branch 'master' into remove_blob_from_eager_blob_object

ca98aad

Merge branch 'master' into remove_blob_from_eager_blob_object

70d2563

Merge branch 'master' into remove_blob_from_eager_blob_object

da8d573

Merge branch 'master' into remove_blob_from_eager_blob_object

d365d43

Signed-off-by: daquexian <daquexian566@gmail.com>

daquexian force-pushed the remove_blob_from_eager_blob_object branch from 11cb68c to d365d43 Compare April 28, 2022 08:53

fix compile error

610967c

Signed-off-by: daquexian <daquexian566@gmail.com>

daquexian added 2 commits April 28, 2022 22:32

Merge branch 'master' into remove_blob_from_eager_blob_object

5bc9db2

Merge branch 'master' into remove_blob_from_eager_blob_object

91f2783

daquexian added the automerge label Apr 29, 2022

mergify bot added 3 commits April 29, 2022 04:10

Merge branch 'master' into remove_blob_from_eager_blob_object

3830cfe

Merge branch 'master' into remove_blob_from_eager_blob_object

8713aaf

Merge branch 'master' into remove_blob_from_eager_blob_object

e57e40c

github-actions bot removed the automerge label Apr 29, 2022

daquexian added the automerge label Apr 29, 2022

daquexian and others added 4 commits April 29, 2022 21:41

Merge branch 'master' into remove_blob_from_eager_blob_object

7055628

Merge branch 'master' into remove_blob_from_eager_blob_object

59e9f34

Merge branch 'master' into remove_blob_from_eager_blob_object

23cfba7

Merge branch 'master' into remove_blob_from_eager_blob_object

fe855c0

github-actions bot removed the automerge label Apr 29, 2022

daquexian added 2 commits May 3, 2022 12:21

Merge branch 'master' into remove_blob_from_eager_blob_object

267f901

Signed-off-by: daquexian <daquexian566@gmail.com>

fix merge bug

75d06ba

Signed-off-by: daquexian <daquexian566@gmail.com>

daquexian merged commit 3a75d14 into master May 3, 2022

daquexian deleted the remove_blob_from_eager_blob_object branch May 3, 2022 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove almost all usage of `Blob` in EagerBlobObject #7895

Remove almost all usage of `Blob` in EagerBlobObject #7895

daquexian commented Mar 25, 2022 •

edited

Loading

github-actions bot commented Mar 29, 2022

Flowingsun007 commented Mar 29, 2022 •

edited

Loading

Flowingsun007 Mar 30, 2022

daquexian Mar 30, 2022

Flowingsun007 Mar 30, 2022

github-actions bot commented Apr 15, 2022

github-actions bot commented Apr 16, 2022

github-actions bot commented Apr 19, 2022

github-actions bot commented Apr 28, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented May 3, 2022

lixinqi commented May 3, 2022

daquexian commented May 3, 2022

Remove almost all usage of Blob in EagerBlobObject #7895

Remove almost all usage of Blob in EagerBlobObject #7895

Conversation

daquexian commented Mar 25, 2022 • edited Loading

github-actions bot commented Mar 29, 2022

Flowingsun007 commented Mar 29, 2022 • edited Loading

swin性能测试

环境

数据

结论

Flowingsun007 Mar 30, 2022

Choose a reason for hiding this comment

daquexian Mar 30, 2022

Choose a reason for hiding this comment

Flowingsun007 Mar 30, 2022

Choose a reason for hiding this comment

github-actions bot commented Apr 15, 2022

github-actions bot commented Apr 16, 2022

github-actions bot commented Apr 19, 2022

github-actions bot commented Apr 28, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented Apr 29, 2022

github-actions bot commented May 3, 2022

lixinqi commented May 3, 2022

daquexian commented May 3, 2022

Remove almost all usage of `Blob` in EagerBlobObject #7895

Remove almost all usage of `Blob` in EagerBlobObject #7895

daquexian commented Mar 25, 2022 •

edited

Loading

Flowingsun007 commented Mar 29, 2022 •

edited

Loading