Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted memory-related fixes for HostManager, EE, RecSys #3411

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions include/glow/ExecutionEngine/ExecutionEngine.h
Expand Up @@ -47,7 +47,7 @@ class ExecutionEngine final {
/// this resets the ExecutionEngine.
std::string backendName_ = "";

/// Size of device memory, if 0 device default is used.
/// Size of device memory in bytes, if 0 device default is used.
uint64_t deviceMemory_{0};

/// The HostManager for executing the compiled functions.
Expand All @@ -61,7 +61,10 @@ class ExecutionEngine final {
void runInternal(ExecutionContext &context, llvm::StringRef name);

public:
ExecutionEngine(llvm::StringRef backend = "Interpreter");
/// Constructor for an ExecutionEngine with \p backend and memory \p
/// deviceMemory in bytes.
ExecutionEngine(llvm::StringRef backend = "Interpreter",
uint64_t deviceMemory = 0);
jfix71 marked this conversation as resolved.
Show resolved Hide resolved

~ExecutionEngine();

Expand Down
6 changes: 4 additions & 2 deletions lib/ExecutionEngine/ExecutionEngine.cpp
Expand Up @@ -27,16 +27,17 @@

using namespace glow;

ExecutionEngine::ExecutionEngine(llvm::StringRef backend) {
ExecutionEngine::ExecutionEngine(llvm::StringRef backend, uint64_t deviceMemory)
: deviceMemory_(deviceMemory) {
setBackendName(backend);
}

/// Set the code generator to the given \p backend.
void ExecutionEngine::setBackendName(llvm::StringRef backend) {
clear();
module_.reset(new Module);
rawModule_ = module_.get();
backendName_ = backend;
clear();

if (hostManager_) {
EXIT_ON_ERR(hostManager_->clearHost());
Expand All @@ -61,6 +62,7 @@ void ExecutionEngine::clear() {
EXIT_ON_ERR(hostManager_->clearHost());
}
compiledFunctions_.clear();
module_.reset(nullptr);
}

void glow::updateInputPlaceholders(PlaceholderBindings &bindings,
Expand Down
14 changes: 6 additions & 8 deletions lib/Runtime/HostManager/HostManager.cpp
Expand Up @@ -226,20 +226,18 @@ llvm::Error HostManager::clearHost() {
DCHECK_EQ(activeRequestCount_, 0)
<< "All requests should be finished when shutting down HostManager.";

// Remove all networks from the host and device(s).
while (networks_.size() != 0) {
RETURN_IF_ERR(removeNetwork(networks_.begin()->first));
}

// Now it's safe to stop the DeviceManagers.
std::lock_guard<std::mutex> networkLock(networkLock_);
OneErrOnly errContainer;
for (auto &it : devices_) {
errContainer.set(it.second->stop());
}

for (auto &network : networks_) {
for (auto &node : network.second.dag.nodes) {
for (auto device : node->deviceIDs) {
devices_[device]->evictNetwork(node->name);
}
}
}
networks_.clear();
return errContainer.get();
}

Expand Down
5 changes: 4 additions & 1 deletion tests/unittests/BackendTestUtils.h
Expand Up @@ -74,7 +74,10 @@ DECLARE_STATELESS_BACKEND_TEST(BackendStatelessTest, std::tuple<std::string>);

class BackendTest : public BackendStatelessTest {
public:
BackendTest() : mod_(EE_.getModule()) { F_ = mod_.createFunction("main"); }
BackendTest(uint64_t deviceMemory = 0)
: EE_(getBackendName(), deviceMemory), mod_(EE_.getModule()) {
F_ = mod_.createFunction("main");
}

protected:
ExecutionEngine EE_{getBackendName()};
Expand Down