Fuse instruction #7399
Merged
Fuse instruction #7399
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: daquexian <[email protected]>
Signed-off-by: daquexian <[email protected]>
…ched_nccl stream; 2) VirtualMachineEngine::local_pending_msg_list_;
Signed-off-by: daquexian <[email protected]>
…low into refactor_release_tensor
… = false; 3) InstructionType::OnDispatch
…o fuse_instruction
daquexian
reviewed
Jan 30, 2022
| intrusive::ChannelStatus status = mut_pending_instruction_list()->MoveTo(&tmp_list); | ||
| intrusive::ChannelStatus status = (mut_pending_instruction_list()->*Move)(&tmp_list); | ||
| *cnt = tmp_list.size(); | ||
| if (*cnt == 0) { return status; } |
There was a problem hiding this comment.
这个 size_t* cnt 是什么作用呢，看起来可以是一个局部变量
| @@ -42,6 +50,7 @@ class InstructionType { | |||
| virtual void Compute(VirtualMachineEngine* vm, InstructionMsg* instr_msg) const { | |||
| LOG(FATAL) << "UNIMPLEMENTED"; | |||
| } | |||
| virtual void ComputeInFuseMode(InstructionMsg* instr_msg) const { LOG(FATAL) << "UNIMPLEMENTED"; } | |||
There was a problem hiding this comment.
应为Compute的参数类型是Instruction*，现在还需要ComputeInFuseMode，等single-client的代码移除后，InstructionMsg会和Instruction合并，到时这里就可以统一成Compute了。
daquexian
reviewed
Feb 7, 2022
oneflow/core/vm/stream_type.h
Outdated
| @@ -61,6 +61,7 @@ class StreamType { | |||
| int64_t this_machine_id) const = 0; | |||
|
|
|||
| virtual bool OnSchedulerThread() const = 0; | |||
| virtual bool EnableInstructionFuse() const { return false; } | |||
daquexian
approved these changes
Feb 7, 2022
BBuf
reviewed
Feb 7, 2022
| int cnt = kLimit; | ||
| InstructionMsgList pending_instr_msgs; | ||
| constexpr static int kPendingHandleWindow = 10; | ||
| GetRewritedPendingInstructionsByWindowSize(kPendingHandleWindow, &pending_instr_msgs); |
There was a problem hiding this comment.
这里的意思是最多融合10条指令？融合的指令数以及依靠融合指令带来的launch开销是怎么确定呢？假如有100个tensor它们经过一个concat变成一个tensor，那么在Dispath concat这个LocaCallOpKernel的之后会有100条可以fuse的指令，这种情况是不是可以把100条都融合在一起？
There was a problem hiding this comment.
这个10肯定影响融合的上限。但这个值的本意与指令融合没有关系。从变量名kPendingHandleWindow可以看出，它目的是为了让scheduler不要过分的只关注pending的情况，每处理完10条指令，应该尝试看看有没有其他stream上指令已完成。
kPendingHandleWindow这个值肯定可以调成100，但意义不大。因为并不是越大越好，越大就越可能导致其他stream的饥饿。
BBuf
approved these changes
Feb 7, 2022
oneflow-ci-bot
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
Speed stats: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vm 为每条指令的执行做了不少支撑工作，这个开销与kernel launch相比往往不能忽略。本pr尝试将多条指令融合在一起，在vm里一起被调度。这样，vm对多条指令的调度成本就平摊开来了。