Skip to content

Commit

Permalink
Revert "Custom Operator Profiling Enhancement (apache#15210)"
Browse files Browse the repository at this point in the history
This reverts commit 92fce90.
  • Loading branch information
Rohit Kumar Srivastava committed Sep 6, 2019
1 parent 69217f5 commit c7147d9
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 363 deletions.
5 changes: 0 additions & 5 deletions src/engine/naive_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "../profiler/profiler.h"
#include "./openmp.h"
#include "../common/object_pool.h"
#include "../profiler/custom_op_profiler.h"

namespace mxnet {
namespace engine {
Expand Down Expand Up @@ -164,10 +163,6 @@ class NaiveEngine final : public Engine {
};
std::unique_ptr<NaiveOpr, decltype(opr_deleter)> opr(nullptr, opr_deleter);
const bool profiling = opr_name && profiler->IsProfiling(profiler::Profiler::kImperative);
// GenerateDisplayName() will return a pointer to the correct name of the operator
const char* display_name = profiling ?
profiler::CustomOpProfiler::Get()->GenerateDisplayName(opr_name) :
opr_name;
if (profiling) {
opr.reset(NewOperator(exec_fun, const_vars, mutable_vars,
prop, display_name)->Cast<NaiveOpr>());
Expand Down
10 changes: 3 additions & 7 deletions src/engine/threaded_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,8 @@ void ThreadedEngine::DeleteOperator(OprHandle op) {

void ThreadedEngine::Push(OprHandle op, Context exec_ctx, int priority, bool profiling) {
BulkFlush();

ThreadedOpr* threaded_opr = ThreadedOpr::CastFromBase(op);
if (profiling) {
threaded_opr->opr_name =
profiler::CustomOpProfiler::Get()->GenerateDisplayName(threaded_opr->opr_name);
}
OprBlock* opr_block = OprBlock::New();
opr_block->opr = threaded_opr;

Expand Down Expand Up @@ -336,10 +333,9 @@ void ThreadedEngine::PushAsync(AsyncFn fn, Context exec_ctx,
<< device_count_;
}
#endif
const bool profiling = profiler_->IsProfiling(profiler::Profiler::kImperative);
ThreadedOpr *opr = NewOperator(std::move(fn), const_vars, mutable_vars,
prop, opr_name, wait);
ThreadedOpr *opr = NewOperator(std::move(fn), const_vars, mutable_vars, prop, opr_name, wait);
opr->temporary = true;
const bool profiling = profiler_->IsProfiling(profiler::Profiler::kImperative);
Push(opr, exec_ctx, priority, profiling);
}

Expand Down
1 change: 0 additions & 1 deletion src/engine/threaded_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
#include "../profiler/profiler.h"
#include "./openmp.h"
#include "../common/object_pool.h"
#include "../profiler/custom_op_profiler.h"

namespace mxnet {
namespace engine {
Expand Down
23 changes: 5 additions & 18 deletions src/operator/custom/custom-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
#include <condition_variable>
#include <queue>
#include "../operator_common.h"
#include "../../profiler/custom_op_profiler.h"

namespace mxnet {
namespace op {
Expand Down Expand Up @@ -77,16 +76,9 @@ class CustomOperator {
bool training, const std::vector<NDArray>& arrs,
const std::vector<int>& tags,
const std::unordered_set<int>& output_tags,
const std::vector<NDArray>& outputs,
const std::string op_type = "") {
const std::vector<NDArray>& outputs) {
if (naive_engine_) {
if (profiler::Profiler::Get()->IsProfiling(profiler::Profiler::kImperative)) {
profiler::CustomOpProfiler::Get()->OnCustomBegin(op_type);
func();
profiler::CustomOpProfiler::Get()->OnCustomEnd();
} else {
func();
}
func();
for (size_t i = 0, out_idx = 0; i < arrs.size(); i++) {
if (arrs[i].storage_type() == kDefaultStorage ||
arrs[i].storage_type() == kUndefinedStorage)
Expand All @@ -105,13 +97,7 @@ class CustomOperator {
bool prev_training = Imperative::Get()->set_is_training(training);

try {
if (profiler::Profiler::Get()->IsProfiling(profiler::Profiler::kImperative)) {
profiler::CustomOpProfiler::Get()->OnCustomBegin(op_type);
func();
profiler::CustomOpProfiler::Get()->OnCustomEnd();
} else {
func();
}
func();
} catch (dmlc::Error& e) {
exception_ =
std::make_shared<std::exception_ptr>(std::current_exception());
Expand Down Expand Up @@ -157,7 +143,8 @@ class CustomOperator {

ctx.async_on_complete();
},
ctx.run_ctx.ctx, vars, vars2, FnProperty::kNoSkip, 0, "CustomOperatorWait");
ctx.run_ctx.ctx, vars, vars2, FnProperty::kNoSkip, 0,
"CustomOperator");
});
// increase num_threads if there is not enough threads to execute custom operator
if (q_.size() > num_free_threads_)
Expand Down
4 changes: 2 additions & 2 deletions src/operator/custom/custom.cc
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ void ForwardEx(const OpStatePtr& state, const OpContext& ctx,
static_cast<int>(ctx.is_train),
params.info->contexts[kCustomOpForward]));
},
ctx, false, ctx.is_train, cpys, tags, output_tags, outputs, params.op_type);
ctx, false, ctx.is_train, cpys, tags, output_tags, outputs);
}

void BackwardEx(const OpStatePtr& state, const OpContext& ctx,
Expand Down Expand Up @@ -415,7 +415,7 @@ void BackwardEx(const OpStatePtr& state, const OpContext& ctx,
ptrs.size(), const_cast<void**>(ptrs.data()), const_cast<int*>(tags.data()),
reinterpret_cast<const int*>(req.data()), static_cast<int>(ctx.is_train),
params.info->contexts[kCustomOpBackward]));
}, ctx, false, ctx.is_train, cpys, tags, output_tags, outputs, "_backward_" + params.op_type);
}, ctx, false, ctx.is_train, cpys, tags, output_tags, outputs);
}

// infer storage backward function for custom op which assigns kDefaultStorage for
Expand Down
125 changes: 0 additions & 125 deletions src/profiler/custom_op_profiler.h

This file was deleted.

53 changes: 7 additions & 46 deletions src/profiler/profiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,13 +781,6 @@ struct ProfileTask : public ProfileDuration {
NVTX_ONLY_CODE(nvtx_duration_.reset(new nvtx::NVTXDuration(name)));
}

/*!
* \brief Set the domain
*/
void setDomain(ProfileDomain* domain) {
domain_ = domain;
}

/*!
* \brief Start the profiling scope
*/
Expand Down Expand Up @@ -1109,8 +1102,6 @@ struct ProfileMarker {
VTUNE_ONLY_CODE(std::unique_ptr<vtune::VTuneInstantMarker> vtune_instant_marker_);
};

static ProfileDomain custom_op_domain("Custom Operator");

/*!
* \brief Operator profiler object. Logs as both an independent event and a task in
* the operator domain
Expand Down Expand Up @@ -1162,16 +1153,10 @@ struct ProfileOperator : public ProfileEvent {
: ProfileEvent(name)
, as_task_(name, &domain_)
, name_(name)
, attributes_(attributes)
, profiling_(!IsDeprecatedOperator(name)) {
if (IsSubOperatorOfCustom(name)) {
as_task_.setDomain(&custom_op_domain);
SetCategories(custom_op_domain.name());
} else {
SetCategories(domain_.name());
}
, attributes_(attributes) {
// make as_task_ not to add stat to AggregateStats; otherwise we will add twice
as_task_.enableAggregateStats(false);
SetCategories(domain_.name());
}
/*!
* \brief Start the profiling scope
Expand All @@ -1181,19 +1166,15 @@ struct ProfileOperator : public ProfileEvent {
void startForDevice(mxnet::Context::DeviceType dev_type, uint32_t dev_id) {
dev_type_ = dev_type;
dev_id_ = dev_id;
if (profiling_) {
ProfileEvent::start();
as_task_.start();
}
ProfileEvent::start();
as_task_.start();
}
/*!
* \brief Stop the profiling scope
*/
void stop() override {
if (profiling_) {
as_task_.stop();
ProfileEvent::stop();
}
as_task_.stop();
ProfileEvent::stop();
}

/*!
Expand All @@ -1218,11 +1199,7 @@ struct ProfileOperator : public ProfileEvent {
if (attributes) {
name_.append(attributes->to_string().c_str());
}
if (IsSubOperatorOfCustom(name)) {
categories_.set(custom_op_domain.name());
} else {
categories_.set("operator");
}
categories_.set("operator");
items_[kStart].timestamp_ = start_time;
items_[kStop].timestamp_ = stop_time;
}
Expand All @@ -1242,20 +1219,6 @@ struct ProfileOperator : public ProfileEvent {
start_time_, ProfileStat::NowInMicrosec(),
attributes_.get());
}
/*!
* \brief Check if this operator is no longer profiled
* Notice that this operator may still be used for e.g synchronization
*/
inline static bool IsDeprecatedOperator(const char* name) {
return strcmp(name, "CustomOperatorWait") == 0 ||
strcmp(name, "Custom") == 0 || strcmp(name, "_backward_Custom") == 0;
}
/*!
* \brief Check if this operator a sub-operator of a custom operator
*/
inline static bool IsSubOperatorOfCustom(const char* name) {
return strstr(name, "::");
}
/*! \brief Also log the operator as a task in the operator domain */
ProfileTask as_task_;
/* !\brief Operator name */
Expand All @@ -1268,8 +1231,6 @@ struct ProfileOperator : public ProfileEvent {
static ProfileDomain domain_;
/*! \brief Optional operator attributes */
std::unique_ptr<Attributes> attributes_;
/*! \brief Whether to profile or not */
const bool profiling_;
};

/*
Expand Down
Loading

0 comments on commit c7147d9

Please sign in to comment.