Pass ownership of Module to HostManager #2572
Conversation
jackm321
force-pushed
the
hostmanager_owns_module
branch
from
4746061
to
b5a44c9
Compare
| DAG dag; | ||
| // list of placeholders owned by the NetworkData. | ||
| PlaceholderList placeholders; | ||
|
|
There was a problem hiding this comment.
So why constants won't be needed in this struct?
There was a problem hiding this comment.
Collect constants will copy them
https://github.com/pytorch/glow/blob/master/include/glow/Backends/BackendUtils.h#L70-L77
|
Why do we need to keep the placeholders and module around after loading the network? It seems storing the placeholders is partially duplicating what the runtimeBundle does. |
|
Module is not kept around it gets thrown out at the end of |
|
Oh I see it allocates symbols out of them. How would that actually work when the Context that's created for each run is created by code that only has original placeholders from compile time? |
|
Discussed this offline with @gcatron, we will keep the Placeholders around during runtime for now as keys into the RuntimeBundle's allocations for the placeholders. In the longer term though, we should switch to replacing placeholders with just their names throughout the runtime since there's no reason to actually keep the placeholders around from compilation other than as a unique identifier. |
| } | ||
|
|
||
| modules_.push_back(std::move(module)); |
There was a problem hiding this comment.
Do we need to store the module here? If so when do we free it?
There was a problem hiding this comment.
Yes good catch, we shouldn't need this now.
jackm321
force-pushed
the
hostmanager_owns_module
branch
from
b5a44c9
to
453658b
Compare
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
jackm321
force-pushed
the
hostmanager_owns_module
branch
3 times, most recently
from
cfd1614
to
1375168
Compare
jackm321
force-pushed
the
hostmanager_owns_module
branch
from
1375168
to
157e224
Compare
jackm321
force-pushed
the
hostmanager_owns_module
branch
from
157e224
to
39f45df
Compare
Description:
Make it so that HostManager and runtime owns the Module. HostManager will take ownership of Module in
addNetworkand keeps the PlaceHolders around internally.Testing:
ninja checkRun resnet-runtime example
Documentation:
comments
Fixes #2567