[Runtime] Allow for parameter sharing in GraphRuntime #3384
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
Summary: In multi-threaded applications where we have multiple inferences on the same model in parallel (consider e.g. a TTS system handling multiple requests), it can be useful to share the parameters of a model amongst these multiple instances. This improves the cache utilization behaviour of the system, as multiple cores can use the same set of weights instead of evicting the identical copies of weights in a shared cache. As the underlying `NDArray` instances in `data_entry_` implement a ref-counted based sharing system, this is a simple modification of the `GraphRuntime::LoadParams` logic to instead copy parameters from an existing GraphRuntime instance. This is a little ugly in that we need both the pre-existing GraphRuntime instance, as well as the 'serialized' params (since we need to know the set of names we should copy), but without imposing additional assumptions (i.e. storing the set of param names in GraphRuntime, and enforcing that shared param names are identical to the parameters set in the preceding `LoadParams` call), this seems unavoidable. Test Plan: Unit test added.
|
LGTM @ajtulloch Can you take a look at ci failure? |
huajsj
reviewed
Jun 18, 2019
| std::vector<std::string> names; | ||
| CHECK(strm->Read(&names)) << "Invalid parameters file format"; | ||
| uint64_t sz; | ||
| strm->Read(&sz); |
There was a problem hiding this comment.
here may need a 'CHECK' just like line 189 and 193 did.
| CHECK_LT(eid, data_entry_.size()); | ||
| CHECK_EQ(data_entry_[eid].use_count(), 1); | ||
| data_entry_[eid] = other.GetInput(GetInputIndex(names[i])); | ||
| CHECK_GT(data_entry_[eid].use_count(), 1); |
There was a problem hiding this comment.
for consistency, 205,206 208 may need a "message" just like other check did.
There was a problem hiding this comment.
data_entry_[eid].use_count() should get increase 1 after one reference, this check just check if the count > 1 seems like not logically complete. how about some logic like following?
int prev_idx = other.GetInput(in_idx).use_count;
...
CHECK_EQ(data_entry_[eid].use_count(), prev_idx + 1);
| uint32_t eid = this->entry_id(input_nodes_[in_idx], 0); | ||
| CHECK_LT(eid, data_entry_.size()); | ||
| CHECK_EQ(data_entry_[eid].use_count(), 1); | ||
| data_entry_[eid] = other.GetInput(GetInputIndex(names[i])); |
There was a problem hiding this comment.
for reuse variable purpose , here should be data_entry_[eid] = other.GetInput(in_idx).
one more question is 'is other.GetInputIndex(names[i]) always equal this->GetInputIndex(names[i]) ?', here i guess the logic should be other.GetInput(other.GetInputIndex(names[i])), if they are same we can reuse in_idx.
|
@ajtulloch can you address @huajsj 's comment? |
tqchen
added
status: accepted
and removed
status: need review
status: need update
|
Thanks @ajtulloch @huajsj , this PR is now merged |
wweic
pushed a commit
to wweic/tvm
that referenced
this pull request
Jun 26, 2019
Summary: In multi-threaded applications where we have multiple inferences on the same model in parallel (consider e.g. a TTS system handling multiple requests), it can be useful to share the parameters of a model amongst these multiple instances. This improves the cache utilization behaviour of the system, as multiple cores can use the same set of weights instead of evicting the identical copies of weights in a shared cache. As the underlying `NDArray` instances in `data_entry_` implement a ref-counted based sharing system, this is a simple modification of the `GraphRuntime::LoadParams` logic to instead copy parameters from an existing GraphRuntime instance. This is a little ugly in that we need both the pre-existing GraphRuntime instance, as well as the 'serialized' params (since we need to know the set of names we should copy), but without imposing additional assumptions (i.e. storing the set of param names in GraphRuntime, and enforcing that shared param names are identical to the parameters set in the preceding `LoadParams` call), this seems unavoidable. Test Plan: Unit test added.
wweic
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Jun 27, 2019
Summary: In multi-threaded applications where we have multiple inferences on the same model in parallel (consider e.g. a TTS system handling multiple requests), it can be useful to share the parameters of a model amongst these multiple instances. This improves the cache utilization behaviour of the system, as multiple cores can use the same set of weights instead of evicting the identical copies of weights in a shared cache. As the underlying `NDArray` instances in `data_entry_` implement a ref-counted based sharing system, this is a simple modification of the `GraphRuntime::LoadParams` logic to instead copy parameters from an existing GraphRuntime instance. This is a little ugly in that we need both the pre-existing GraphRuntime instance, as well as the 'serialized' params (since we need to know the set of names we should copy), but without imposing additional assumptions (i.e. storing the set of param names in GraphRuntime, and enforcing that shared param names are identical to the parameters set in the preceding `LoadParams` call), this seems unavoidable. Test Plan: Unit test added.
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.
Summary:
In multi-threaded applications where we have multiple inferences on the
same model in parallel (consider e.g. a TTS system handling multiple
requests), it can be useful to share the parameters of a model amongst
these multiple instances. This improves the cache utilization behaviour
of the system, as multiple cores can use the same set of weights instead
of evicting the identical copies of weights in a shared cache.
As the underlying
NDArrayinstances indata_entry_implement aref-counted based sharing system, this is a simple modification of the
GraphRuntime::LoadParamslogic to instead copy parameters from anexisting GraphRuntime instance. This is a little ugly in that we need
both the pre-existing GraphRuntime instance, as well as the 'serialized'
params (since we need to know the set of names we should copy), but
without imposing additional assumptions (i.e. storing the set of param
names in GraphRuntime, and enforcing that shared param names are
identical to the parameters set in the preceding
LoadParamscall),this seems unavoidable.
Test Plan:
Unit test added.