From ef586c6fca20756d41962dac1bc0821e4431ad5f Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Tue, 24 Apr 2018 01:49:42 -0700 Subject: [PATCH 1/7] move from varhandle to variable header --- paddle/fluid/framework/CMakeLists.txt | 2 +- paddle/fluid/framework/details/var_handle.h | 1 + paddle/fluid/framework/details/var_id.h | 15 ++++++++ paddle/fluid/framework/scope.cc | 9 +++-- paddle/fluid/framework/scope.h | 20 +++++++--- paddle/fluid/framework/type_defs.h | 6 ++- paddle/fluid/framework/variable.h | 41 +++++++++++++++++++++ paddle/fluid/framework/variable_test.cc | 19 +++++++++- paddle/fluid/platform/profiler.cc | 9 +++++ paddle/fluid/platform/profiler.h | 2 + 10 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 paddle/fluid/framework/details/var_id.h diff --git a/paddle/fluid/framework/CMakeLists.txt b/paddle/fluid/framework/CMakeLists.txt index 340b891e41671d..bbcb1102ed0e2e 100644 --- a/paddle/fluid/framework/CMakeLists.txt +++ b/paddle/fluid/framework/CMakeLists.txt @@ -33,7 +33,7 @@ cc_test(variable_test SRCS variable_test.cc) cc_library(threadpool SRCS threadpool.cc DEPS enforce) cc_test(threadpool_test SRCS threadpool_test.cc DEPS threadpool) -cc_library(scope SRCS scope.cc DEPS glog threadpool) +cc_library(scope SRCS scope.cc DEPS glog threadpool profiler) cc_test(scope_test SRCS scope_test.cc DEPS scope) cc_library(data_device_transform SRCS data_device_transform.cc DEPS tensor) diff --git a/paddle/fluid/framework/details/var_handle.h b/paddle/fluid/framework/details/var_handle.h index 2b887c67e6fc6e..5feb3f15bc3858 100644 --- a/paddle/fluid/framework/details/var_handle.h +++ b/paddle/fluid/framework/details/var_handle.h @@ -68,6 +68,7 @@ struct DummyVarHandle : public VarHandleBase { std::string DebugString() const override; }; + } // namespace details } // namespace framework } // namespace paddle diff --git a/paddle/fluid/framework/details/var_id.h b/paddle/fluid/framework/details/var_id.h new file mode 100644 index 00000000000000..d69ad91ebb460c --- /dev/null +++ b/paddle/fluid/framework/details/var_id.h @@ -0,0 +1,15 @@ +#pragma once +#include + +namespace paddle { +namespace framework { +struct VarId { + explicit VarId(const std::string& name) : name(name), unique_id(-1) {} + VarId(const std::string& name, int id) : name(name), unique_id(id) {} + std::string name; + /*default -1 if uninitialized*/ + int unique_id; +}; + +} // namespace framework +} // namespace paddle diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 194df3e4a8b507..5cd46846bee0df 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -18,6 +18,7 @@ limitations under the License. */ #include #include "glog/logging.h" #include "paddle/fluid/framework/threadpool.h" +#include "paddle/fluid/platform/profiler.h" #include "paddle/fluid/string/printf.h" DEFINE_bool(benchmark, false, @@ -43,7 +44,7 @@ Scope& Scope::NewScope() const { return *kids_.back(); } -Variable* Scope::Var(const std::string& name) { +Variable* Scope::Var(const VarUUID& name) { auto* v = FindVarLocally(name); if (v != nullptr) return v; v = new Variable(); @@ -53,7 +54,7 @@ Variable* Scope::Var(const std::string& name) { return v; } -Variable* Scope::Var(std::string* name) { +Variable* Scope::Var(VarUUID* name) { auto var_name = string::Sprintf("%p.%d", this, vars_.size()); if (name != nullptr) { *name = var_name; @@ -61,7 +62,8 @@ Variable* Scope::Var(std::string* name) { return Var(var_name); } -Variable* Scope::FindVar(const std::string& name) const { +Variable* Scope::FindVar(const VarUUID& name) const { + platform::RecordEvent event("Scope"); auto var = FindVarLocally(name); if (var != nullptr) { return var; @@ -135,6 +137,7 @@ std::string Scope::Rename(const std::string& origin_name) const { } Variable* Scope::FindVarLocally(const std::string& name) const { + platform::RecordEvent event("Scope"); auto it = vars_.find(name); if (it != vars_.end()) return it->second; return nullptr; diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h index c8cb70549f1d13..f1bb0e5224af6a 100644 --- a/paddle/fluid/framework/scope.h +++ b/paddle/fluid/framework/scope.h @@ -47,16 +47,17 @@ class Scope { Scope& NewScope() const; /// Create a variable with given name if it doesn't exist. - Variable* Var(const std::string& name); + Variable* Var(const VarUUID& id); /// Create a variable with a scope-unique name. - Variable* Var(std::string* name = nullptr); + Variable* Var(VarUUID* id = nullptr); - void EraseVars(const std::vector& var_names); + /// EraseVars in scope. + void EraseVars(const std::vector& var_names); /// Find a variable in the scope or any of its ancestors. Returns /// nullptr if cannot find. - Variable* FindVar(const std::string& name) const; + Variable* FindVar(const VarUUID& name) const; const Scope* parent() const { return parent_; } @@ -78,13 +79,20 @@ class Scope { // Rename variable to a new name and return the new name std::string Rename(const std::string& origin_name) const; - Variable* FindVarLocally(const std::string& name) const; + Variable* FindVarLocally(const VarUUID& id) const; private: // Call Scope::NewScope for a sub-scope. explicit Scope(Scope const* parent) : parent_(parent) {} - mutable std::unordered_map vars_; + // mutable std::unordered_map vars_; + struct Hasher { + size_t operator() (const VarUUID& id) const { + return id.unique_id; + } + }; + mutable std::unordered_map vars_; + mutable std::list kids_; Scope const* parent_{nullptr}; diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index 4879209ece9fdf..b9e91736f8ce55 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -21,6 +21,7 @@ limitations under the License. */ #include #include #include "paddle/fluid/platform/variant.h" +#include "paddle/fluid/framework/details/var_handle.h" namespace paddle { namespace framework { @@ -29,7 +30,10 @@ class OpDesc; class InferShapeContext; class BlockDesc; -using VariableNameMap = std::map>; +using details::VarUUID; +class VarUUID; +// using VariableNameMap = std::map>; +using VariableNameMap = std::map>; // The order should be as same as framework.proto using Attribute = diff --git a/paddle/fluid/framework/variable.h b/paddle/fluid/framework/variable.h index 87ddfe2ff9abfa..435ee417b317cf 100644 --- a/paddle/fluid/framework/variable.h +++ b/paddle/fluid/framework/variable.h @@ -16,12 +16,15 @@ #include #include #include +#include // for call_once + #include "paddle/fluid/platform/enforce.h" namespace paddle { namespace framework { + class Variable { public: template @@ -91,5 +94,43 @@ class Variable { const std::string* name_; }; +/// variable unique_id generator, threadsafe singleton. +class UUIDGenerator { +public: + int operator()() {return engine();} + UUIDGenerator& Instance() { + std::call_once(once_flag, &UUIDGenerator::InitOnce); + return *g; + } +private: + static void InitOnce() {g = new UUIDGenerator();} + UUIDGenerator() { + engine.seed(std::random_device()()); + } + std::minstd_rand engine; + static UUIDGenerator *g; + static std::once_flag once_flag; +}; + +UUIDGenerator::UUIDGenerator* g = nullptr; +std::once_flag UUIDGenerator::once_flag; + +// a runtime unique variable identity. +struct VarUUID : public VarHandleBase { + explicit VarUUID(const std::string& name) : name(name), unique_id(-1) {} + VarUUID(const std::string& name, int id) : name(name), unique_id(id) {} + std::string DebugString() const override; + std::string name; + int unique_id; /*default -1 if uninitialized*/ +}; + +inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { + os << var.name; + if (VLOG_IS_ON(5)) { + os << "[" << var.unique_id << "]"; + } + return os; +} + } // namespace framework } // namespace paddle diff --git a/paddle/fluid/framework/variable_test.cc b/paddle/fluid/framework/variable_test.cc index c5c1d215f4a6af..ac1062e87a20bb 100644 --- a/paddle/fluid/framework/variable_test.cc +++ b/paddle/fluid/framework/variable_test.cc @@ -12,15 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include +#include #include "gtest/gtest.h" #include "paddle/fluid/framework/variable.h" -TEST(Variable, GetMutable) { - using paddle::framework::Variable; +using paddle::framework::Variable; +TEST(Variable, GetMutable) { struct Tensor { int content_; }; @@ -39,3 +41,16 @@ TEST(Variable, GetMutable) { const std::string& ss = v->Get(); EXPECT_EQ("hello", ss); } + +TEST(UUIDGenerator, Generate) { + constexpr int count = 10; + std::vector ids; + UUIDGenerator gen = UUIDGenerator::Instance(); + for (int i=0; i < count; ++i) { + ids.push_back(gen()); + } + for(auto& id : ids) { + std::cout << id << " "; + } + std::cout << endl; +} diff --git a/paddle/fluid/platform/profiler.cc b/paddle/fluid/platform/profiler.cc index 412cdda286c3a7..5878534c4d136e 100644 --- a/paddle/fluid/platform/profiler.cc +++ b/paddle/fluid/platform/profiler.cc @@ -176,6 +176,15 @@ RecordEvent::RecordEvent(const std::string& name, const DeviceContext* dev_ctx) SetCurAnnotation(name_); } +RecordEvent::RecordEvent(const std::string& name) : start_ns_(PosixInNsec()) { + if (g_state == ProfilerState::kDisabled) return; + platform::CPUPlace place; + dev_ctx_ = DeviceContextPool::Instance().Get(place); + name_ = name; + PushEvent(name_, dev_ctx_); + SetCurAnnotation(name_); +} + RecordEvent::~RecordEvent() { if (g_state == ProfilerState::kDisabled) return; DeviceTracer* tracer = GetDeviceTracer(); diff --git a/paddle/fluid/platform/profiler.h b/paddle/fluid/platform/profiler.h index b07427c8f6903e..37f80edc4631df 100644 --- a/paddle/fluid/platform/profiler.h +++ b/paddle/fluid/platform/profiler.h @@ -71,6 +71,8 @@ void PushEvent(const std::string& name, const DeviceContext* dev_ctx); void PopEvent(const std::string& name, const DeviceContext* dev_ctx); struct RecordEvent { + explicit RecordEvent(const std::string& name); + RecordEvent(const std::string& name, const DeviceContext* dev_ctx); ~RecordEvent(); From 97b766c62135c4c6761519371990e16c5c84fc58 Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Tue, 24 Apr 2018 22:04:53 -0700 Subject: [PATCH 2/7] "add operator fix" --- .pre-commit-config.yaml | 8 ++--- paddle/fluid/framework/details/var_handle.cc | 6 ++++ paddle/fluid/framework/details/var_handle.h | 29 ++++++++++++++++ paddle/fluid/framework/operator.cc | 16 ++++----- paddle/fluid/framework/operator.h | 36 ++++++++++---------- paddle/fluid/framework/scope.cc | 26 +++++++------- paddle/fluid/framework/scope.h | 7 +--- paddle/fluid/framework/type_defs.h | 2 -- paddle/fluid/framework/variable.h | 33 +++++------------- 9 files changed, 89 insertions(+), 74 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6140340890c0e5..9acb0515e868e7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,16 +1,16 @@ repos: - repo: https://github.com/Lucas-C/pre-commit-hooks.git - sha: v1.0.1 + rev: v1.1.5 hooks: - id: remove-crlf files: (?!.*third_party)^.*$ | (?!.*book)^.*$ - repo: https://github.com/PaddlePaddle/mirrors-yapf.git - sha: 0d79c0c469bab64f7229c9aca2b1186ef47f0e37 + rev: v0.16.2 hooks: - id: yapf files: (.*\.(py|bzl)|BUILD|.*\.BUILD|WORKSPACE)$ - repo: https://github.com/pre-commit/pre-commit-hooks - sha: 5bf6c09bfa1297d3692cadd621ef95f1284e33c0 + rev: v1.2.3 hooks: - id: check-added-large-files - id: check-merge-conflict @@ -35,7 +35,7 @@ repos: language: system files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx)$ - repo: https://github.com/PaddlePaddle/pre-commit-golang - sha: 8337620115c25ff8333f1b1a493bd031049bd7c0 + rev: v0.2 hooks: - id: go-fmt types: diff --git a/paddle/fluid/framework/details/var_handle.cc b/paddle/fluid/framework/details/var_handle.cc index 6f00abd9473a84..6e267cc4eb0d5f 100644 --- a/paddle/fluid/framework/details/var_handle.cc +++ b/paddle/fluid/framework/details/var_handle.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "paddle/fluid/framework/details/var_handle.h" +#include "paddle/fluid/framework/variable.h" namespace paddle { namespace framework { @@ -27,6 +28,11 @@ std::string VarHandle::DebugString() const { } std::string DummyVarHandle::DebugString() const { return "dummy"; } + +VarUUID::VarUUID(const std::strign& name) : name(name) { + unique_id = UUIDGenerator::Instance()(name); +} + } // namespace details } // namespace framework } // namespace paddle diff --git a/paddle/fluid/framework/details/var_handle.h b/paddle/fluid/framework/details/var_handle.h index 5feb3f15bc3858..cedce45c834335 100644 --- a/paddle/fluid/framework/details/var_handle.h +++ b/paddle/fluid/framework/details/var_handle.h @@ -17,6 +17,7 @@ #include #include #include +#include #include "paddle/fluid/platform/place.h" @@ -68,6 +69,34 @@ struct DummyVarHandle : public VarHandleBase { std::string DebugString() const override; }; +// a runtime unique variable identity. +struct VarUUID { + explicit VarUUID(const std::string& name); + VarUUID(const std::string& name, int id) : name(name), unique_id(id) {} + bool operator==(const VarUUID& rhs) const { + return unique_id == rhs.unique_id; + } + bool operator!=(const VarUUID& rhs) const { + return unique_id != rhs.unique_id; + } + std::string name; + int unique_id; /*default -1 if uninitialized*/ +}; + +struct VarUUIDHash { + size_t operator() (const VarUUID& id) const { + return id.unique_id; + } +}; + +inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { + os << var.name; + if (VLOG_IS_ON(5)) { + os << "[" << var.unique_id << "]"; + } + return os; +} + } // namespace details } // namespace framework diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index f97bd0827428fe..c513d72d0696b9 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -93,7 +93,7 @@ void OperatorBase::Run(const Scope& scope, const platform::Place& place) { RunImpl(scope, place); } -std::string OperatorBase::Input(const std::string& name) const { +VarUUID OperatorBase::Input(const std::string& name) const { auto& ins = Inputs(name); PADDLE_ENFORCE_LE(ins.size(), 1UL, "Operator %s's input %s should contain only one variable.", @@ -101,7 +101,7 @@ std::string OperatorBase::Input(const std::string& name) const { return ins.empty() ? kEmptyVarName : ins[0]; } -const std::vector& OperatorBase::Inputs( +const std::vector& OperatorBase::Inputs( const std::string& name) const { auto it = inputs_.find(name); PADDLE_ENFORCE(it != inputs_.end(), "Operator %s does not have the input %s.", @@ -109,7 +109,7 @@ const std::vector& OperatorBase::Inputs( return it->second; } -std::string OperatorBase::Output(const std::string& name) const { +VarUUID OperatorBase::Output(const std::string& name) const { auto& outs = Outputs(name); PADDLE_ENFORCE_LE(outs.size(), 1UL, "Operator %s's output %s should contain only one variable.", @@ -117,7 +117,7 @@ std::string OperatorBase::Output(const std::string& name) const { return outs.empty() ? kEmptyVarName : outs[0]; } -const std::vector& OperatorBase::Outputs( +const std::vector& OperatorBase::Outputs( const std::string& name) const { auto it = outputs_.find(name); PADDLE_ENFORCE(it != outputs_.end(), @@ -191,8 +191,8 @@ OperatorBase::OperatorBase(const std::string& type, CheckAllInputOutputSet(); } -std::vector OperatorBase::InputVars() const { - std::vector ret_val; +std::vector OperatorBase::InputVars() const { + std::vector ret_val; for (auto& o : inputs_) { ret_val.reserve(ret_val.size() + o.second.size()); ret_val.insert(ret_val.end(), o.second.begin(), o.second.end()); @@ -200,8 +200,8 @@ std::vector OperatorBase::InputVars() const { return ret_val; } -std::vector OperatorBase::OutputVars(bool has_intermediate) const { - std::vector ret_val; +std::vector OperatorBase::OutputVars(bool has_intermediate) const { + std::vector ret_val; if (has_intermediate) { // push all outputs into ret_val for (auto& o : outputs_) { diff --git a/paddle/fluid/framework/operator.h b/paddle/fluid/framework/operator.h index b7a7c69b4c8493..5d0f2d85e3ac0f 100644 --- a/paddle/fluid/framework/operator.h +++ b/paddle/fluid/framework/operator.h @@ -109,19 +109,19 @@ class OperatorBase { const VariableNameMap& Outputs() const { return outputs_; } //! Get a input with argument's name described in `op_proto` - std::string Input(const std::string& name) const; + VarUUID Input(const std::string& name) const; //! Get a input which has multiple variables. - const std::vector& Inputs(const std::string& name) const; + const std::vector& Inputs(const std::string& name) const; - std::vector InputVars() const; + std::vector InputVars() const; //! Get a output with argument's name described in `op_proto` - std::string Output(const std::string& name) const; + VarUUID Output(const std::string& name) const; //! Get an output which has multiple variables. //! TODO add a vector_view to prevent memory copy. - const std::vector& Outputs(const std::string& name) const; + const std::vector& Outputs(const std::string& name) const; - virtual std::vector OutputVars(bool has_intermediate) const; + virtual std::vector OutputVars(bool has_intermediate) const; const std::string& Type() const { return type_; } void SetType(const std::string& type) { type_ = type; } @@ -207,12 +207,12 @@ class ExecutionContext { const Variable* InputVar(const std::string& name) const { auto ipt = op_.Input(name); - return ipt == kEmptyVarName ? nullptr : scope_.FindVar(ipt); + return ipt == VarUUID(kEmptyVarName) ? nullptr : scope_.FindVar(ipt); } Variable* OutputVar(const std::string& name) const { auto opt = op_.Output(name); - return opt == kEmptyVarName ? nullptr : scope_.FindVar(opt); + return opt == VarUUID(kEmptyVarName) ? nullptr : scope_.FindVar(opt); } const std::vector MultiInputVar( @@ -221,9 +221,9 @@ class ExecutionContext { std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [this](const std::string& name) { - return name == kEmptyVarName ? nullptr - : scope_.FindVar(name); + [this](const VarUUID& name) { + return name == VarUUID(kEmptyVarName) ? nullptr + : scope_.FindVar(name); }); return res; } @@ -233,9 +233,9 @@ class ExecutionContext { std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [this](const std::string& name) { - return name == kEmptyVarName ? nullptr - : scope_.FindVar(name); + [this](const VarUUID& name) { + return name == VarUUID(kEmptyVarName) ? nullptr + : scope_.FindVar(name); }); return res; } @@ -258,7 +258,7 @@ class ExecutionContext { std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [&](const std::string& sub_name) { + [&](const VarUUID& sub_name) { auto var = scope_.FindVar(sub_name); return var == nullptr ? nullptr : &var->Get(); }); @@ -271,7 +271,7 @@ class ExecutionContext { std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [&](const std::string& sub_name) { + [&](const VarUUID& sub_name) { auto var = scope_.FindVar(sub_name); return var == nullptr ? nullptr : var->GetMutable(); }); @@ -312,12 +312,12 @@ class ExecutionContext { #endif //! Get actual name vector for this input. - const std::vector& Inputs(const std::string& name) const { + const std::vector& Inputs(const std::string& name) const { return op_.Inputs(name); } //! Get actual name vector for this output. - const std::vector& Outputs(const std::string& name) const { + const std::vector& Outputs(const std::string& name) const { return op_.Outputs(name); } diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 5cd46846bee0df..514aa0582ab61f 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -15,7 +15,7 @@ limitations under the License. */ #include "paddle/fluid/framework/scope.h" #include // for unique_ptr -#include +#include #include "glog/logging.h" #include "paddle/fluid/framework/threadpool.h" #include "paddle/fluid/platform/profiler.h" @@ -48,18 +48,19 @@ Variable* Scope::Var(const VarUUID& name) { auto* v = FindVarLocally(name); if (v != nullptr) return v; v = new Variable(); - vars_[name] = v; + vars_[VarUUID(name)] = v; VLOG(3) << "Create variable " << name; - v->name_ = &(vars_.find(name)->first); + v->name_ = &(vars_.find(name)->first.name); return v; } Variable* Scope::Var(VarUUID* name) { auto var_name = string::Sprintf("%p.%d", this, vars_.size()); + VarUUID id(var_name); if (name != nullptr) { - *name = var_name; + *name = id; } - return Var(var_name); + return Var(id); } Variable* Scope::FindVar(const VarUUID& name) const { @@ -88,7 +89,7 @@ std::vector Scope::LocalVarNames() const { std::vector known_vars; known_vars.reserve(this->vars_.size()); for (auto& p : vars_) { - known_vars.emplace_back(p.first); + known_vars.emplace_back(p.first.name); } return known_vars; } @@ -106,8 +107,8 @@ void Scope::DeleteScope(Scope* scope) { } } -void Scope::EraseVars(const std::vector& var_names) { - std::set var_set(var_names.begin(), var_names.end()); +void Scope::EraseVars(const std::vector& var_names) { + std::unordered_set var_set(var_names.begin(), var_names.end()); for (auto it = vars_.begin(); it != vars_.end();) { if (var_set.find(it->first) != var_set.end()) { delete it->second; @@ -120,13 +121,14 @@ void Scope::EraseVars(const std::vector& var_names) { void Scope::Rename(const std::string& origin_name, const std::string& new_name) const { - auto origin_it = vars_.find(origin_name); + auto origin_it = vars_.find(VarUUID(origin_name)); PADDLE_ENFORCE(origin_it != vars_.end(), "Cannot find original variable with name %s", origin_name); - auto new_it = vars_.find(new_name); + VarUUID new_var = VarUUID(new_name); + auto new_it = vars_.find(new_var); PADDLE_ENFORCE(new_it == vars_.end(), "The variable with name %s is already in the scope", new_name); - vars_[new_name] = origin_it->second; + vars_[new_var] = origin_it->second; vars_.erase(origin_it); } @@ -136,7 +138,7 @@ std::string Scope::Rename(const std::string& origin_name) const { return var_name; } -Variable* Scope::FindVarLocally(const std::string& name) const { +Variable* Scope::FindVarLocally(const VarUUID& name) const { platform::RecordEvent event("Scope"); auto it = vars_.find(name); if (it != vars_.end()) return it->second; diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h index f1bb0e5224af6a..81eead500c2dd6 100644 --- a/paddle/fluid/framework/scope.h +++ b/paddle/fluid/framework/scope.h @@ -86,12 +86,7 @@ class Scope { explicit Scope(Scope const* parent) : parent_(parent) {} // mutable std::unordered_map vars_; - struct Hasher { - size_t operator() (const VarUUID& id) const { - return id.unique_id; - } - }; - mutable std::unordered_map vars_; + mutable std::unordered_map vars_; mutable std::list kids_; Scope const* parent_{nullptr}; diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index b9e91736f8ce55..e0d85245cb17c5 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -31,8 +31,6 @@ class InferShapeContext; class BlockDesc; using details::VarUUID; -class VarUUID; -// using VariableNameMap = std::map>; using VariableNameMap = std::map>; // The order should be as same as framework.proto diff --git a/paddle/fluid/framework/variable.h b/paddle/fluid/framework/variable.h index 435ee417b317cf..8c64df9e55548b 100644 --- a/paddle/fluid/framework/variable.h +++ b/paddle/fluid/framework/variable.h @@ -18,8 +18,8 @@ #include #include // for call_once - #include "paddle/fluid/platform/enforce.h" +#include "paddle/fluid/framework/details/var_handle.h" namespace paddle { namespace framework { @@ -94,43 +94,28 @@ class Variable { const std::string* name_; }; +using details::VarUUID; +using details::VarUUIDHash; /// variable unique_id generator, threadsafe singleton. class UUIDGenerator { public: - int operator()() {return engine();} + int operator()(const std::string& name) {return hasher(name);} UUIDGenerator& Instance() { std::call_once(once_flag, &UUIDGenerator::InitOnce); return *g; } private: static void InitOnce() {g = new UUIDGenerator();} - UUIDGenerator() { - engine.seed(std::random_device()()); + explicit UUIDGenerator() { } - std::minstd_rand engine; - static UUIDGenerator *g; + std::hash hasher; + static UUIDGenerator* g; static std::once_flag once_flag; + DISABLE_COPY_AND_ASSIGN(UUIDGenerator); }; -UUIDGenerator::UUIDGenerator* g = nullptr; +UUIDGenerator* UUIDGenerator::g = nullptr; std::once_flag UUIDGenerator::once_flag; -// a runtime unique variable identity. -struct VarUUID : public VarHandleBase { - explicit VarUUID(const std::string& name) : name(name), unique_id(-1) {} - VarUUID(const std::string& name, int id) : name(name), unique_id(id) {} - std::string DebugString() const override; - std::string name; - int unique_id; /*default -1 if uninitialized*/ -}; - -inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { - os << var.name; - if (VLOG_IS_ON(5)) { - os << "[" << var.unique_id << "]"; - } - return os; -} - } // namespace framework } // namespace paddle From d387a2cf68aad40610f101ffff7473ffc3da8ded Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Tue, 24 Apr 2018 22:07:50 -0700 Subject: [PATCH 3/7] "update config" --- paddle/fluid/framework/details/var_handle.h | 7 ++----- paddle/fluid/framework/operator.h | 10 ++++++---- paddle/fluid/framework/scope.cc | 3 ++- paddle/fluid/framework/type_defs.h | 2 +- paddle/fluid/framework/variable.h | 18 ++++++++---------- 5 files changed, 19 insertions(+), 21 deletions(-) diff --git a/paddle/fluid/framework/details/var_handle.h b/paddle/fluid/framework/details/var_handle.h index cedce45c834335..cef3621ff4629d 100644 --- a/paddle/fluid/framework/details/var_handle.h +++ b/paddle/fluid/framework/details/var_handle.h @@ -15,9 +15,9 @@ #pragma once #include #include +#include #include #include -#include #include "paddle/fluid/platform/place.h" @@ -84,9 +84,7 @@ struct VarUUID { }; struct VarUUIDHash { - size_t operator() (const VarUUID& id) const { - return id.unique_id; - } + size_t operator()(const VarUUID& id) const { return id.unique_id; } }; inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { @@ -97,7 +95,6 @@ inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { return os; } - } // namespace details } // namespace framework } // namespace paddle diff --git a/paddle/fluid/framework/operator.h b/paddle/fluid/framework/operator.h index 5d0f2d85e3ac0f..2d465fe549b408 100644 --- a/paddle/fluid/framework/operator.h +++ b/paddle/fluid/framework/operator.h @@ -222,8 +222,9 @@ class ExecutionContext { res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), [this](const VarUUID& name) { - return name == VarUUID(kEmptyVarName) ? nullptr - : scope_.FindVar(name); + return name == VarUUID(kEmptyVarName) + ? nullptr + : scope_.FindVar(name); }); return res; } @@ -234,8 +235,9 @@ class ExecutionContext { res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), [this](const VarUUID& name) { - return name == VarUUID(kEmptyVarName) ? nullptr - : scope_.FindVar(name); + return name == VarUUID(kEmptyVarName) + ? nullptr + : scope_.FindVar(name); }); return res; } diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 514aa0582ab61f..64a73b50ce0cbe 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -108,7 +108,8 @@ void Scope::DeleteScope(Scope* scope) { } void Scope::EraseVars(const std::vector& var_names) { - std::unordered_set var_set(var_names.begin(), var_names.end()); + std::unordered_set var_set(var_names.begin(), + var_names.end()); for (auto it = vars_.begin(); it != vars_.end();) { if (var_set.find(it->first) != var_set.end()) { delete it->second; diff --git a/paddle/fluid/framework/type_defs.h b/paddle/fluid/framework/type_defs.h index e0d85245cb17c5..40d18af1b4c51e 100644 --- a/paddle/fluid/framework/type_defs.h +++ b/paddle/fluid/framework/type_defs.h @@ -20,8 +20,8 @@ limitations under the License. */ #include #include #include -#include "paddle/fluid/platform/variant.h" #include "paddle/fluid/framework/details/var_handle.h" +#include "paddle/fluid/platform/variant.h" namespace paddle { namespace framework { diff --git a/paddle/fluid/framework/variable.h b/paddle/fluid/framework/variable.h index 8c64df9e55548b..9db9e93c82dedb 100644 --- a/paddle/fluid/framework/variable.h +++ b/paddle/fluid/framework/variable.h @@ -14,17 +14,16 @@ #pragma once #include +#include #include #include -#include // for call_once -#include "paddle/fluid/platform/enforce.h" #include "paddle/fluid/framework/details/var_handle.h" +#include "paddle/fluid/platform/enforce.h" namespace paddle { namespace framework { - class Variable { public: template @@ -70,7 +69,7 @@ class Variable { // parameter of Variable. template struct PlaceholderImpl : public Placeholder { - PlaceholderImpl(T* ptr) : ptr_(ptr), type_(typeid(T)) {} + explicit PlaceholderImpl(T* ptr) : ptr_(ptr), type_(typeid(T)) {} virtual const std::type_info& Type() const { return type_; } virtual void* Ptr() const { return static_cast(ptr_.get()); } @@ -98,16 +97,15 @@ using details::VarUUID; using details::VarUUIDHash; /// variable unique_id generator, threadsafe singleton. class UUIDGenerator { -public: - int operator()(const std::string& name) {return hasher(name);} + public: + int operator()(const std::string& name) { return hasher(name); } UUIDGenerator& Instance() { std::call_once(once_flag, &UUIDGenerator::InitOnce); return *g; } -private: - static void InitOnce() {g = new UUIDGenerator();} - explicit UUIDGenerator() { - } + + private: + static void InitOnce() { g = new UUIDGenerator(); } std::hash hasher; static UUIDGenerator* g; static std::once_flag once_flag; From f87fc04a0532d611b32cd4b6817088d60cdb29da Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Tue, 24 Apr 2018 23:47:44 -0700 Subject: [PATCH 4/7] 'add scope test' --- paddle/fluid/framework/details/var_handle.cc | 6 ++-- paddle/fluid/framework/details/var_handle.h | 29 +++++++++++++++++++- paddle/fluid/framework/operator.cc | 19 +++++++------ paddle/fluid/framework/scope_test.cc | 26 +++++++++--------- paddle/fluid/framework/variable.h | 19 ------------- paddle/fluid/pybind/pybind.cc | 7 +++-- 6 files changed, 60 insertions(+), 46 deletions(-) diff --git a/paddle/fluid/framework/details/var_handle.cc b/paddle/fluid/framework/details/var_handle.cc index 6e267cc4eb0d5f..c2101c9794c6f1 100644 --- a/paddle/fluid/framework/details/var_handle.cc +++ b/paddle/fluid/framework/details/var_handle.cc @@ -29,9 +29,9 @@ std::string VarHandle::DebugString() const { std::string DummyVarHandle::DebugString() const { return "dummy"; } -VarUUID::VarUUID(const std::strign& name) : name(name) { - unique_id = UUIDGenerator::Instance()(name); -} +// VarUUID::VarUUID(const std::strign& name) : name(name) { +// unique_id = UUIDGenerator::Instance()(name); +// } } // namespace details } // namespace framework diff --git a/paddle/fluid/framework/details/var_handle.h b/paddle/fluid/framework/details/var_handle.h index cef3621ff4629d..82cc4a79a7b770 100644 --- a/paddle/fluid/framework/details/var_handle.h +++ b/paddle/fluid/framework/details/var_handle.h @@ -69,9 +69,35 @@ struct DummyVarHandle : public VarHandleBase { std::string DebugString() const override; }; +/// variable unique_id generator, threadsafe singleton. +class UUIDGenerator { +public: + int operator()(const std::string& name) { return hasher(name); } + int Hash(const std::string& name) { return hasher(name); } + static UUIDGenerator& Instance() { + std::call_once(once_flag, &UUIDGenerator::InitOnce); + return *g; + } + +private: + static void InitOnce() { g = new UUIDGenerator(); } + UUIDGenerator() {} + std::hash hasher; + static UUIDGenerator* g; + static std::once_flag once_flag; + DISABLE_COPY_AND_ASSIGN(UUIDGenerator); +}; + +UUIDGenerator* UUIDGenerator::g = nullptr; +std::once_flag UUIDGenerator::once_flag; + // a runtime unique variable identity. struct VarUUID { - explicit VarUUID(const std::string& name); + explicit VarUUID(const std::string& name) : name(name) { + // UUIDGenerator& gen = UUIDGenerator::Instance(); + // unique_id = gen(name); + unique_id = UUIDGenerator::Instance()(name); + } VarUUID(const std::string& name, int id) : name(name), unique_id(id) {} bool operator==(const VarUUID& rhs) const { return unique_id == rhs.unique_id; @@ -95,6 +121,7 @@ inline std::ostream& operator<<(std::ostream& os, const VarUUID& var) { return os; } + } // namespace details } // namespace framework } // namespace paddle diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index c513d72d0696b9..e16437871ed911 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -14,6 +14,7 @@ limitations under the License. */ #include #include +#include #include #include "paddle/fluid/framework/data_transform.h" @@ -46,7 +47,7 @@ proto::VarType::Type GetDataTypeOfVar(const Variable* var) { } } -static DDim GetDims(const Scope& scope, const std::string& name, +static DDim GetDims(const Scope& scope, const VarUUID& name, bool get_actual_dim = false) { Variable* var = scope.FindVar(name); if (var == nullptr) { @@ -66,7 +67,7 @@ static DDim GetDims(const Scope& scope, const std::string& name, } } -static LoD GetLoD(const Scope& scope, const std::string& name) { +static LoD GetLoD(const Scope& scope, const VarUUID& name) { Variable* var = scope.FindVar(name); auto default_lod = LoD({{}}); @@ -98,7 +99,7 @@ VarUUID OperatorBase::Input(const std::string& name) const { PADDLE_ENFORCE_LE(ins.size(), 1UL, "Operator %s's input %s should contain only one variable.", type_, name); - return ins.empty() ? kEmptyVarName : ins[0]; + return ins.empty() ? VarUUID(kEmptyVarName) : ins[0]; } const std::vector& OperatorBase::Inputs( @@ -114,7 +115,7 @@ VarUUID OperatorBase::Output(const std::string& name) const { PADDLE_ENFORCE_LE(outs.size(), 1UL, "Operator %s's output %s should contain only one variable.", type_, name); - return outs.empty() ? kEmptyVarName : outs[0]; + return outs.empty() ? VarUUID(kEmptyVarName) : outs[0]; } const std::vector& OperatorBase::Outputs( @@ -244,11 +245,13 @@ void OperatorBase::CheckAllInputOutputSet() const { void OperatorBase::GenerateTemporaryNames() { static std::atomic gUniqId(0UL); for (auto& output : outputs_) { - for (auto& output_name : output.second) { - if (output_name == kTempVarName) { + for (auto& output_id : output.second) { + if (output_id.name == kTempVarName) { + std::string output_name = ""; output_name += type_; output_name += "@"; output_name += std::to_string(gUniqId.fetch_add(1)); + output_id = VarUUID(output_name); } } } @@ -294,7 +297,7 @@ const std::vector ExecutionContext::MultiInput( std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [&](const std::string& sub_name) { + [&](const VarUUID& sub_name) { auto var = scope_.FindVar(sub_name); return var == nullptr ? nullptr : GetTensorFromVar(var); }); @@ -314,7 +317,7 @@ std::vector ExecutionContext::MultiOutput( std::vector res; res.reserve(names.size()); std::transform(names.begin(), names.end(), std::back_inserter(res), - [&](const std::string& sub_name) { + [&](const VarUUID& sub_name) { auto var = scope_.FindVar(sub_name); return var == nullptr ? nullptr : GetMutableTensorFromVar(var); diff --git a/paddle/fluid/framework/scope_test.cc b/paddle/fluid/framework/scope_test.cc index ebf8178a8319cd..96d18cbd7d99a4 100644 --- a/paddle/fluid/framework/scope_test.cc +++ b/paddle/fluid/framework/scope_test.cc @@ -24,33 +24,33 @@ TEST(Scope, VarsShadowing) { Scope& ss1 = s.NewScope(); Scope& ss2 = s.NewScope(); - Variable* v0 = s.Var("a"); - Variable* v1 = ss1.Var("a"); + Variable* v0 = s.Var(VarUUID("a")); + Variable* v1 = ss1.Var(VarUUID("a")); EXPECT_NE(v0, v1); - EXPECT_EQ(v0, s.FindVar("a")); - EXPECT_EQ(v1, ss1.FindVar("a")); - EXPECT_EQ(v0, ss2.FindVar("a")); + EXPECT_EQ(v0, s.FindVar(VarUUID("a"))); + EXPECT_EQ(v1, ss1.FindVar(VarUUID("a"))); + EXPECT_EQ(v0, ss2.FindVar(VarUUID("a"))); } TEST(Scope, FindVar) { Scope s; Scope& ss = s.NewScope(); - EXPECT_EQ(nullptr, s.FindVar("a")); - EXPECT_EQ(nullptr, ss.FindVar("a")); + EXPECT_EQ(nullptr, s.FindVar(VarUUID("a"))); + EXPECT_EQ(nullptr, ss.FindVar(VarUUID("a"))); - ss.Var("a"); + ss.Var(VarUUID("a")); - EXPECT_EQ(nullptr, s.FindVar("a")); - EXPECT_NE(nullptr, ss.FindVar("a")); + EXPECT_EQ(nullptr, s.FindVar(VarUUID("a"))); + EXPECT_NE(nullptr, ss.FindVar(VarUUID("a"))); } TEST(Scope, FindScope) { Scope s; Scope& ss = s.NewScope(); - Variable* v = s.Var("a"); + Variable* v = s.Var(VarUUID("a")); EXPECT_EQ(&s, s.FindScope(v)); EXPECT_EQ(&s, ss.FindScope(v)); @@ -58,7 +58,7 @@ TEST(Scope, FindScope) { TEST(Scope, GetAllNames) { Scope s; - Variable* v = s.Var("a"); + Variable* v = s.Var(VarUUID("a")); EXPECT_EQ(&s, s.FindScope(v)); std::vector ans = s.LocalVarNames(); @@ -67,5 +67,5 @@ TEST(Scope, GetAllNames) { str += var; } - EXPECT_STREQ("a", str.c_str()); + EXPECT_STREQ(VarUUID("a"), str.c_str()); } diff --git a/paddle/fluid/framework/variable.h b/paddle/fluid/framework/variable.h index 9db9e93c82dedb..e43bafbf27ef43 100644 --- a/paddle/fluid/framework/variable.h +++ b/paddle/fluid/framework/variable.h @@ -95,25 +95,6 @@ class Variable { using details::VarUUID; using details::VarUUIDHash; -/// variable unique_id generator, threadsafe singleton. -class UUIDGenerator { - public: - int operator()(const std::string& name) { return hasher(name); } - UUIDGenerator& Instance() { - std::call_once(once_flag, &UUIDGenerator::InitOnce); - return *g; - } - - private: - static void InitOnce() { g = new UUIDGenerator(); } - std::hash hasher; - static UUIDGenerator* g; - static std::once_flag once_flag; - DISABLE_COPY_AND_ASSIGN(UUIDGenerator); -}; - -UUIDGenerator* UUIDGenerator::g = nullptr; -std::once_flag UUIDGenerator::once_flag; } // namespace framework } // namespace paddle diff --git a/paddle/fluid/pybind/pybind.cc b/paddle/fluid/pybind/pybind.cc index 64d92cac7eca10..5901b7a7281279 100644 --- a/paddle/fluid/pybind/pybind.cc +++ b/paddle/fluid/pybind/pybind.cc @@ -249,10 +249,13 @@ All parameter, weight, gradient are variables in Paddle. py::class_(m, "Scope", "") .def("var", [](Scope &self, const std::string &name) -> Variable * { - return self.Var(name); + return self.Var(VarUUID(name)); }, py::return_value_policy::reference) - .def("find_var", &Scope::FindVar, py::return_value_policy::reference) + .def("find_var",[](const std::string& name) -> Variable* { + return self.FindVar(VarUUID(name)); + } + , py::return_value_policy::reference) .def(py::init<>()) .def("new_scope", [](Scope &self) -> Scope * { return &self.NewScope(); }, py::return_value_policy::reference) From a5b910025d563cb373244fe6ac8a86c874f3cb46 Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Wed, 25 Apr 2018 00:08:08 -0700 Subject: [PATCH 5/7] 'add testing' --- paddle/fluid/framework/operator.cc | 8 ++++---- paddle/fluid/framework/scope.cc | 20 ++++++++++++++++++++ paddle/fluid/framework/scope.h | 4 ++++ paddle/fluid/framework/shape_inference.h | 4 ++-- paddle/fluid/operators/prefetch_op.cc | 4 ++-- paddle/fluid/operators/send_barrier_op.cc | 4 ++-- paddle/fluid/operators/while_op.cc | 2 +- 7 files changed, 35 insertions(+), 11 deletions(-) diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index e16437871ed911..6fe3367faae031 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -355,7 +355,7 @@ class RuntimeInferShapeContext : public InferShapeContext { PADDLE_ENFORCE_EQ(length, 1UL, "Input %s should not have more than one inputs", name); auto ipt = ins[0]; - auto* var = ipt == kEmptyVarName ? nullptr : scope_.FindVar(ipt); + auto* var = ipt == VarUUID(kEmptyVarName) ? nullptr : scope_.FindVar(ipt); return var != nullptr; } @@ -368,7 +368,7 @@ class RuntimeInferShapeContext : public InferShapeContext { PADDLE_ENFORCE_EQ(length, 1UL, "Output %s should not have more than one inputs", name); auto ipt = outs[0]; - auto* var = ipt == kEmptyVarName ? nullptr : scope_.FindVar(ipt); + auto* var = ipt == VarUUID(kEmptyVarName) ? nullptr : scope_.FindVar(ipt); return var != nullptr; } @@ -400,12 +400,12 @@ class RuntimeInferShapeContext : public InferShapeContext { AttrReader Attrs() const override { return AttrReader(op_.Attrs()); } - const std::vector& Inputs( + const std::vector& Inputs( const std::string& name) const override { return op_.Inputs(name); } - const std::vector& Outputs( + const std::vector& Outputs( const std::string& name) const override { return op_.Outputs(name); } diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc index 64a73b50ce0cbe..4d78daadc16cb0 100644 --- a/paddle/fluid/framework/scope.cc +++ b/paddle/fluid/framework/scope.cc @@ -44,6 +44,10 @@ Scope& Scope::NewScope() const { return *kids_.back(); } +Variable* Scope::Var(const std::string& name) { + return Var(VarUUID(name)); +} + Variable* Scope::Var(const VarUUID& name) { auto* v = FindVarLocally(name); if (v != nullptr) return v; @@ -54,6 +58,14 @@ Variable* Scope::Var(const VarUUID& name) { return v; } +Variable* Scope::Var(std::string* name) { + auto var_name = string::Sprintf("%p.%d", this, vars_.size()); + if (name != nullptr) { + *name = var_name; + } + return Var(var_name); +} + Variable* Scope::Var(VarUUID* name) { auto var_name = string::Sprintf("%p.%d", this, vars_.size()); VarUUID id(var_name); @@ -63,6 +75,10 @@ Variable* Scope::Var(VarUUID* name) { return Var(id); } +Variable* Scope::FindVar(const std::string& name) const { + return FindVar(VarUUID(name)); +} + Variable* Scope::FindVar(const VarUUID& name) const { platform::RecordEvent event("Scope"); auto var = FindVarLocally(name); @@ -139,6 +155,10 @@ std::string Scope::Rename(const std::string& origin_name) const { return var_name; } +Variable* Scope::FindVarLocally(const std::string& name) const { + return FindVarLocally(name); +} + Variable* Scope::FindVarLocally(const VarUUID& name) const { platform::RecordEvent event("Scope"); auto it = vars_.find(name); diff --git a/paddle/fluid/framework/scope.h b/paddle/fluid/framework/scope.h index 81eead500c2dd6..58f8a81fbca2e4 100644 --- a/paddle/fluid/framework/scope.h +++ b/paddle/fluid/framework/scope.h @@ -48,9 +48,11 @@ class Scope { /// Create a variable with given name if it doesn't exist. Variable* Var(const VarUUID& id); + Variable* Var(const std::string& name); /// Create a variable with a scope-unique name. Variable* Var(VarUUID* id = nullptr); + Variable* Var(std::string* name = nullptr); /// EraseVars in scope. void EraseVars(const std::vector& var_names); @@ -58,6 +60,7 @@ class Scope { /// Find a variable in the scope or any of its ancestors. Returns /// nullptr if cannot find. Variable* FindVar(const VarUUID& name) const; + Variable* FindVar(const std::string& name) const; const Scope* parent() const { return parent_; } @@ -80,6 +83,7 @@ class Scope { std::string Rename(const std::string& origin_name) const; Variable* FindVarLocally(const VarUUID& id) const; + Variable* FindVarLocally(const std::string& name) const; private: // Call Scope::NewScope for a sub-scope. diff --git a/paddle/fluid/framework/shape_inference.h b/paddle/fluid/framework/shape_inference.h index bc02d700da5186..490937a19267f6 100644 --- a/paddle/fluid/framework/shape_inference.h +++ b/paddle/fluid/framework/shape_inference.h @@ -49,9 +49,9 @@ class InferShapeContext { void SetReaderDims(const std::string &name, const std::vector &dims); virtual AttrReader Attrs() const = 0; - virtual const std::vector &Inputs( + virtual const std::vector &Inputs( const std::string &name) const = 0; - virtual const std::vector &Outputs( + virtual const std::vector &Outputs( const std::string &name) const = 0; virtual void ShareLoD(const std::string &in, const std::string &out, diff --git a/paddle/fluid/operators/prefetch_op.cc b/paddle/fluid/operators/prefetch_op.cc index f9ae01ab5d2972..79583ad86c0bab 100644 --- a/paddle/fluid/operators/prefetch_op.cc +++ b/paddle/fluid/operators/prefetch_op.cc @@ -42,10 +42,10 @@ class PrefetchOp : public framework::OperatorBase { auto& ctx = *pool.Get(place); auto client_var_name = Output("RPCClient"); - PADDLE_ENFORCE_NOT_NULL(scope.FindVar(client_var_name), + PADDLE_ENFORCE_NOT_NULL(scope.FindVar(VarUUID(client_var_name)), "Can not find variable '%s' in the scope.", client_var_name); - auto* client_var = scope.FindVar(client_var_name); + auto* client_var = scope.FindVar(VarUUID(client_var_name)); detail::RPCClient* rpc_client = client_var->GetMutable(); for (size_t i = 0; i < ins.size(); i++) { diff --git a/paddle/fluid/operators/send_barrier_op.cc b/paddle/fluid/operators/send_barrier_op.cc index 12b844daaa3316..2664156a0d9f2f 100644 --- a/paddle/fluid/operators/send_barrier_op.cc +++ b/paddle/fluid/operators/send_barrier_op.cc @@ -38,10 +38,10 @@ class SendBarrierOp : public framework::OperatorBase { std::vector eps = Attr>("endpoints"); auto client_var_name = Output("RPCClient"); - PADDLE_ENFORCE_NOT_NULL(scope.FindVar(client_var_name), + PADDLE_ENFORCE_NOT_NULL(scope.FindVar(VarUUID(client_var_name)), "Can not find variable '%s' in the scope.", client_var_name); - auto* client_var = scope.FindVar(client_var_name); + auto* client_var = scope.FindVar(VarUUID(client_var_name)); detail::RPCClient* rpc_client = client_var->GetMutable(); // need to wait before sending send_barrier message diff --git a/paddle/fluid/operators/while_op.cc b/paddle/fluid/operators/while_op.cc index 8b62b242cf8745..06be229fed9e07 100644 --- a/paddle/fluid/operators/while_op.cc +++ b/paddle/fluid/operators/while_op.cc @@ -131,7 +131,7 @@ class WhileGradOp : public framework::OperatorBase { VLOG(8) << "Linking outside " << outside_og_name << " --> inside " << inside_og_name; auto &og_outside = - detail::Ref(scope.FindVar(outside_og_name), + detail::Ref(scope.FindVar(VarUUID(outside_og_name)), "Cannot find Outside Gradient %s", outside_og_name); auto &og_inside = detail::Ref(cur_scope.Var(inside_og_name), From 796633903c3c2f48b08a77258dd66b5198c07507 Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Wed, 25 Apr 2018 02:02:32 -0700 Subject: [PATCH 6/7] "re-commit" --- .pre-commit-config.yaml | 8 ++++++++ paddle/fluid/framework/details/var_id.h | 15 --------------- 2 files changed, 8 insertions(+), 15 deletions(-) delete mode 100644 paddle/fluid/framework/details/var_id.h diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2f55b26d5a0cc8..9acb0515e868e7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -26,6 +26,14 @@ repos: entry: bash ./.clang_format.hook -i language: system files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx|proto)$ +- repo: local + hooks: + - id: cpplint-cpp-source + name: cpplint + description: Check C++ code style using cpplint.py. + entry: bash ./tools/codestyle/cpplint_pre_commit.hook + language: system + files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx)$ - repo: https://github.com/PaddlePaddle/pre-commit-golang rev: v0.2 hooks: diff --git a/paddle/fluid/framework/details/var_id.h b/paddle/fluid/framework/details/var_id.h deleted file mode 100644 index d69ad91ebb460c..00000000000000 --- a/paddle/fluid/framework/details/var_id.h +++ /dev/null @@ -1,15 +0,0 @@ -#pragma once -#include - -namespace paddle { -namespace framework { -struct VarId { - explicit VarId(const std::string& name) : name(name), unique_id(-1) {} - VarId(const std::string& name, int id) : name(name), unique_id(id) {} - std::string name; - /*default -1 if uninitialized*/ - int unique_id; -}; - -} // namespace framework -} // namespace paddle From 6c83e7d9e1985edb823896bfb959beb22dc3be63 Mon Sep 17 00:00:00 2001 From: dzhwinter Date: Mon, 30 Apr 2018 06:45:54 -0700 Subject: [PATCH 7/7] "staged shape inference" --- paddle/fluid/framework/details/var_handle.h | 15 +++------------ paddle/fluid/framework/shape_inference.cc | 12 ++++++------ paddle/fluid/framework/shape_inference.h | 5 +++-- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/paddle/fluid/framework/details/var_handle.h b/paddle/fluid/framework/details/var_handle.h index ade3e6b4823000..6d7f360f9127bc 100644 --- a/paddle/fluid/framework/details/var_handle.h +++ b/paddle/fluid/framework/details/var_handle.h @@ -77,30 +77,21 @@ struct DummyVarHandle : public VarHandleBase { /// variable unique_id generator, threadsafe singleton. class UUIDGenerator { public: + UUIDGenerator() {} int operator()(const std::string& name) { return hasher(name); } - int Hash(const std::string& name) { return hasher(name); } static UUIDGenerator& Instance() { - std::call_once(once_flag, &UUIDGenerator::InitOnce); - return *g; + static UUIDGenerator g; + return g; } private: - static void InitOnce() { g = new UUIDGenerator(); } - UUIDGenerator() {} std::hash hasher; - static UUIDGenerator* g; - static std::once_flag once_flag; DISABLE_COPY_AND_ASSIGN(UUIDGenerator); }; -UUIDGenerator* UUIDGenerator::g = nullptr; -std::once_flag UUIDGenerator::once_flag; - // a runtime unique variable identity. struct VarUUID { explicit VarUUID(const std::string& name) : name(name) { - // UUIDGenerator& gen = UUIDGenerator::Instance(); - // unique_id = gen(name); unique_id = UUIDGenerator::Instance()(name); } VarUUID(const std::string& name, int id) : name(name), unique_id(id) {} diff --git a/paddle/fluid/framework/shape_inference.cc b/paddle/fluid/framework/shape_inference.cc index ddff2c7c261746..0651d298034c74 100644 --- a/paddle/fluid/framework/shape_inference.cc +++ b/paddle/fluid/framework/shape_inference.cc @@ -23,7 +23,7 @@ namespace paddle { namespace framework { DDim InferShapeContext::GetInputDim(const std::string &name) const { - const std::vector &arg_names = Inputs(name); + auto &arg_names = Inputs(name); PADDLE_ENFORCE_EQ(arg_names.size(), 1UL, "Input(%s) should hold one element, but now it holds %d", name, arg_names.size()); @@ -32,13 +32,13 @@ DDim InferShapeContext::GetInputDim(const std::string &name) const { std::vector InferShapeContext::GetInputsDim( const std::string &name) const { - const std::vector &arg_names = Inputs(name); + auto &arg_names = Inputs(name); return GetDims(arg_names); } std::vector InferShapeContext::GetReaderDims( const std::string &name) const { - const std::vector &arg_names = Inputs(name); + auto &arg_names = Inputs(name); PADDLE_ENFORCE_EQ( arg_names.size(), 1UL, "Reader input '%s' should hold one element, but now it holds %d", name, @@ -68,7 +68,7 @@ void InferShapeContext::SetOutputsDim(const std::string &name, void InferShapeContext::SetReaderDims(const std::string &name, const std::vector &dims) { - const std::vector &arg_names = Outputs(name); + auto &arg_names = Outputs(name); PADDLE_ENFORCE_EQ( arg_names.size(), 1UL, "Reader output '%s' should hold one element, but now it holds %d", name, @@ -78,7 +78,7 @@ void InferShapeContext::SetReaderDims(const std::string &name, std::vector InferShapeContext::GetInputVarPtrs( const std::string &name) { - const std::vector arg_names = Inputs(name); + auto &arg_names = Inputs(name); std::vector res; res.reserve(arg_names.size()); std::transform( @@ -89,7 +89,7 @@ std::vector InferShapeContext::GetInputVarPtrs( std::vector InferShapeContext::GetOutputVarPtrs( const std::string &name) { - const std::vector arg_names = Outputs(name); + auto &arg_names = Outputs(name); std::vector res; res.reserve(arg_names.size()); std::transform( diff --git a/paddle/fluid/framework/shape_inference.h b/paddle/fluid/framework/shape_inference.h index b763f7bde7697f..46c8feec001584 100644 --- a/paddle/fluid/framework/shape_inference.h +++ b/paddle/fluid/framework/shape_inference.h @@ -51,8 +51,9 @@ class InferShapeContext { void SetReaderDims(const std::string &name, const std::vector &dims); virtual AttrReader Attrs() const = 0; - virtual const std::vector &Inputs(const std::string &name) const = 0; - virtual const std::vector &Outputs( + virtual const std::vector &Inputs( + const std::string &name) const = 0; + virtual const std::vector &Outputs( const std::string &name) const = 0; virtual void ShareLoD(const std::string &in, const std::string &out,