Dev regst coloring #965

lixinqi · 2018-06-26T08:33:13Z

No description provided.

…yte size

…IsOnlyOneTopoOrder

…puteSameChainActorIds

* is head regst desc where sharing same mem * fix typo * memory sharing info * interface: add ctrl regst for mem_sharing critical section * find shared mem header to sink and add ctrl regst * refine MakeSetterAddCtrlRegst * move out of loop * pass RegstDescProto* vector instead * move loop out and extract BuildCtrlRegst * use shared_ptr instead of unique_ptr * find or create * refine parameters * directly connected tasks have no need to add guard ctrl regst * modify because of mem_sharing_info changing from optional to required * refine code * fix bug * rename * extract TryConnectWithCtrlRegstDesc and InitCtrlRegstDesc * rename * rename * FindOrCreate from improve to task_node * rename TryConnectWithMemSafeGuardCtrlRegstDesc parameters * check mem_sharing_info.enable_mem_sharing * extract IsInRepeatedField * at(0) to front() * refine IsInRepeatedField * use single template argument T * find all shared mem regst sink comsumer task * adjust semantic * ref capture IsReachable * task_ids captured by ref * remove useless function

…nto dev_regst_coloring

yuanms2 · 2018-06-29T14:51:02Z

oneflow/core/register/register_desc.h

@@ -8,6 +8,8 @@ namespace oneflow {

 const int32_t kMaxRegisterNum = std::numeric_limits<int32_t>::max();

+void InitCtrlRegstDesc(int64_t produced_task_id, RegstDescProto* ctrl_regst_proto);


produced_task_id -> producer_task_id

yuanms2 · 2018-06-29T14:51:22Z

oneflow/core/register/register_desc.cpp

@@ -146,4 +146,19 @@ bool RegstDesc::HasSameBlobDescs(const RegstDesc* rhs) {
  return true;
 }

+void InitCtrlRegstDesc(int64_t produced_task_id, RegstDescProto* ctrl_regst_proto) {


produced_task_id -> producer_task_id

yuanms2 · 2018-06-29T14:53:56Z

oneflow/core/job/improver.h

-  Plan Improve(const Plan& naive_plan, const std::string& act_event_filepath);
+  Plan Improve(const AvailableMemDesc& amd, const Plan& naive_plan,
+               const std::string& act_event_filepath);
+  Plan ImproveMemSharedInfoOnly(const Plan& naive_plan) const;


为什么有的地方是mem shared info，有的地方是mem sharing info

嗯，没有及时改过来

yuanms2 · 2018-06-29T15:39:37Z

oneflow/core/job/improver.cpp

+         && regst_desc.mem_sharing_info().enable_mem_sharing();
+}
+
+bool IsConsumersAndProducerInSameChain(const RegstDescProto& regst_desc,


比较绕，这个不用搞一个集合去判断吧，直接判断是否所有的consumer的 chain id 和 producer chain id 是否相等就可以了吧。

yuanms2 · 2018-06-29T15:45:20Z

oneflow/core/job/improver.cpp

+    const std::function<void(const std::list<const RegstDescProto*>&)>& Handler) {
+  HashMap<int64_t, std::list<const TaskProto*>> chain_id2task_proto;
+  for (const TaskProto& task : plan.task()) {
+    if (Global<IDMgr>::Get()->LocalWorkStreamId4TaskId(task.task_id()) != 0) { continue; }


这是为什么？是只处理每个thread上的第一个stream上的task? 也就是不处理那种independent stream?

independent stream上的内存共享开发人员手动设置，不在这推断

yuanms2 · 2018-06-29T16:02:43Z

oneflow/core/job/improver.cpp

+  ForEachSameColoredChainRegstDescWithConsumer(graph, plan, [&](const RegstDescs& regst_descs) {
+    int32_t used_order_value = 0;
+    mem_sharing_info.set_mem_shared_id(Global<IDMgr>::Get()->NewMemSharedId());
+    graph.SortByProducerTaskTopoOrder(regst_descs, [&](const RegstDescProto* regst_desc) {


感觉这里的逻辑可以简化：利用chain_id和order_in_graph那个值。

yuanms2 · 2018-07-01T05:50:54Z

oneflow/core/graph/plan_task_graph.cpp

+  });
+}
+
+void PlanTaskGraph::AssertThereIsOnlyOneTopoOrder(const HashSet<int64_t>& task_ids) const {


在plan graph里任何时候不要自己做拓扑遍历，全部使用order in graph规定的顺序来做，自然保证永远都是一个顺序。

yuanms2 · 2018-07-01T05:59:41Z

oneflow/core/graph/plan_task_graph.cpp

+  });
+}
+
+void PlanTaskGraph::ComputeLifetimeActorIds(const RegstDescProto* regst_desc,


这里也不需要再拓扑遍历了。

yuanms2 · 2018-07-01T06:00:58Z

oneflow/core/job/improver.cpp

+      });
+}
+
+void ForEachSameColoredChainRegstDescWithConsumer(


这部分代码callback嵌套的太多了，逻辑分散在各处，不容易读懂。

yuanms2 · 2018-07-01T06:41:53Z

oneflow/core/job/improver.cpp

+    const Plan& plan, const std::function<void(const std::list<const RegstDescProto*>&)>& Handler) {
+  HashMap<int64_t, std::list<const RegstDescProto*>> global_work_stream_id2regst_descs;
+  for (const auto& task : plan.task()) {
+    if (Global<IDMgr>::Get()->LocalWorkStreamId4TaskId(task.task_id()) != 0) { continue; }


这个地方也通过在TaskNode set regst sharable false来禁用共享。

yuanms2 · 2018-07-01T06:53:03Z

oneflow/core/graph/regst_lifetime_graph.cpp

+  for (const auto& pair : task_id2intersected_nodes) {
+    for (RegstLifetimeNode* src_node : pair.second) {
+      for (RegstLifetimeNode* dst_node : pair.second) {
+        if (src_node < dst_node) { src_node2dst_nodes[src_node].emplace(dst_node); }


这个地方根据指针大小来排列节点，运行多次的话结果不确定。可以按照RegstLifetimeNode->regst->producer task node 的order_in_graph来排序，这样每次运行结果都一样。

不好，因为src_node和dst_node可能在一个task上。
换成regst_desc_id就行

lixinqi added 30 commits June 4, 2018 21:31

top design of regst coloring

183e8cb

ForEachImprovedMemSharedId

f20d103

ForEachComputeStreamRegstDescs

f5b1890

memory shared task graph

734126b

refactor InitNodes and InitEdges in MemSharedTaskGraph

27fa114

remove memory_shared_task_graph.* files

8479bd4

RegstLifetimePoset

d57d75f

ForEachLayerwiseSameColoredRegstDescIds

b36d6c0

ForEachSameColoredRegstDescIds

0b79e11

more const

e9ff47b

ImproveMemSharedIdOnly

03741e6

Merge branch 'master' into dev_regst_coloring

abe0d2b

refactor InitRegstDescId2RegstDesc => MakeRegstDescId2RegstDesc

6911ea0

refine CalcMemoryConsumed

569d794

rename task_id -> actor_id

bf3972b

rename: IsAnyOneReachable => IsAnyNodeReachableToAncestor

b11c7ea

bugfix: it's possible that two regst_desc has the same lifetime and b…

a71aeb6

…yte size

refine ForEachLayerwiseSameColoredRegstDescIds

8344ac4

AssertThereIsOnlyOneTopoOrder

72efbab

move AssertThereIsOnlyOneTopoOrder to MemSharedTaskGraph::AssertThere…

8036679

…IsOnlyOneTopoOrder

don't assert enable_mem_sharing consistency within a task node

e1dd33d

Merge branch 'master' into dev_regst_coloring

c11a3d1

assert topo order only once for each global work stream

60cf239

Merge branch 'master' into dev_regst_coloring

804fe87

global mem_shared_id

d895d9d

sort regst desc by producer task topo order

9507b68

merge master

e155730

MemSharedTaskGraph => PlanTaskGraph; ComputeSameStreamActorIds => Com…

bede4e5

…puteSameChainActorIds

move PlanTaskGraph,RegstLifetimPosetGraph to single files

70c9d44

refine ForEachImprovedMemSharingInfo

a385a35

lixinqi and others added 4 commits June 22, 2018 17:39

simplify ForEachSameColoredChainRegstDescWithConsumer

3a8c6c3

Merge branch 'master' into dev_regst_coloring

996623a

Merge branch 'dev_regst_coloring' of github.com:Oneflow-Inc/oneflow i…

acf5291

…nto dev_regst_coloring

lixinqi requested a review from willzhang4a58 as a code owner June 26, 2018 08:33

lixinqi added 2 commits June 26, 2018 20:34

bugfix node2excluded_color_ids.at(node) -> node2excluded_color_ids[node]

075cf71

bug fix

237a8d8

yuanms2 reviewed Jun 29, 2018

View reviewed changes

lixinqi added 2 commits June 29, 2018 23:15

Merge branch 'master' into dev_regst_coloring

553266c

produced_task_id => producer_task_id; MemSharedInfo => MemSharingInfo

5ccad89

yuanms2 reviewed Jun 29, 2018

View reviewed changes

lixinqi added 3 commits June 30, 2018 11:13

SortByProducerTaskOrderInGraph

92b3e5e

CHECK_NE(chain_id, -1)

bf90588

Merge branch 'master' into dev_regst_coloring

97d8bb2

yuanms2 reviewed Jul 1, 2018

View reviewed changes

lixinqi and others added 5 commits July 1, 2018 15:27

no topo travel on PlanTaskGraph

c46e203

remove MemSharingInfo

54d7291

delete useless code

ca7ca79

regst coloring always makes the same results

f5f205f

remove local work stream id in improver

4489784

yuanms2 merged commit a36c035 into master Jul 1, 2018

lixinqi deleted the dev_regst_coloring branch July 2, 2018 06:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev regst coloring #965

Dev regst coloring #965

lixinqi commented Jun 26, 2018

yuanms2 Jun 29, 2018

yuanms2 Jun 29, 2018

yuanms2 Jun 29, 2018

lixinqi Jun 29, 2018

yuanms2 Jun 29, 2018

yuanms2 Jun 29, 2018

lixinqi Jun 29, 2018

yuanms2 Jun 29, 2018

yuanms2 Jul 1, 2018

yuanms2 Jul 1, 2018

yuanms2 Jul 1, 2018

yuanms2 Jul 1, 2018

yuanms2 Jul 1, 2018

lixinqi Jul 1, 2018

		@@ -8,6 +8,8 @@ namespace oneflow {

		const int32_t kMaxRegisterNum = std::numeric_limits<int32_t>::max();

		void InitCtrlRegstDesc(int64_t produced_task_id, RegstDescProto* ctrl_regst_proto);

Dev regst coloring #965

Dev regst coloring #965

Conversation

lixinqi commented Jun 26, 2018

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

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