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.

Sign up for GitHub

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

optimize: take action #198

Merged
merged 1 commit into from
Oct 8, 2022
Merged

optimize: take action #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 18 additions & 29 deletions src/commands/dump.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ const DependentMap dependent_map = {
namespace {
uv_thread_t uv_profiling_callback_thread;
ActionMap action_map;
RequestMap request_map;
std::string cpuprofile_filepath = "";
std::string sampling_heapprofile_filepath = "";
std::string heapsnapshot_filepath = "";
Expand Down Expand Up @@ -123,13 +122,6 @@ void DependentActionRunning(DumpAction action, XpfError& err) {
}
}

void TransactionDone(string thread_name, string unique_key, XpfError& err) {
if (request_map.find(unique_key) != request_map.end()) {
err = XpfError::Failure("<%s> %s has been executed by other thread.",
thread_name.c_str(), unique_key.c_str());
}
}

template <typename T>
T* GetProfilingData(void* data, string notify_type, string unique_key) {
Isolate* isolate = Isolate::GetCurrent();
Expand All @@ -140,13 +132,6 @@ T* GetProfilingData(void* data, string notify_type, string unique_key) {
return dump_data;
}

template <typename T>
T* GetDumpData(void* data) {
T* dump_data = static_cast<T*>(data);
if (!dump_data->run_once) dump_data->run_once = true;
return dump_data;
}

void AfterDumpFile(string& filepath, string notify_type, string unique_key) {
Isolate* isolate = Isolate::GetCurrent();
EnvironmentData* env_data = EnvironmentData::GetCurrent(isolate);
Expand All @@ -157,11 +142,17 @@ void AfterDumpFile(string& filepath, string notify_type, string unique_key) {

} // namespace

#define CLEAR_DATA \
DebugT(module_type, env_data->thread_id(), "<%s> %s dump_data cleared.", \
notify_type.c_str(), unique_key.c_str()); \
delete dump_data;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

得看看怎么把这个变成智能指针 -。-

Copy link
Member Author

@hyj1991 hyj1991 Oct 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我也想过这里用智能指针，不过我不太熟悉怕反而写出来 bug，要不合并后你来看看是否可以重构成智能指针？

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 pr 主要目的还是清理掉已经无用的旧逻辑


#define CHECK_ERR(func) \
func; \
if (err.Fail()) { \
DebugT(module_type, env_data->thread_id(), "<%s> %s error: %s", \
notify_type.c_str(), unique_key.c_str(), err.GetErrMessage()); \
CLEAR_DATA; \
return; \
}

Expand All @@ -174,21 +165,8 @@ void HandleAction(v8::Isolate* isolate, void* data, string notify_type) {
// check transaction has been done
XpfError err;
string unique_key = traceid + "::" + Action2String(action);
TransactionDone(notify_type, unique_key, err);
if (err.Fail()) {
DebugT(module_type, env_data->thread_id(), "%s", err.GetErrMessage());
request_map.erase(unique_key);
// clear dump_data
if (dump_data->run_once) {
DebugT(module_type, env_data->thread_id(), "<%s> %s dump_data cleared.",
notify_type.c_str(), unique_key.c_str());
delete dump_data;
}
return;
}

// set action executing flag
request_map.insert(make_pair(unique_key, true));
DebugT(module_type, env_data->thread_id(), "<%s> %s handled.",
notify_type.c_str(), unique_key.c_str());

Expand All @@ -207,7 +185,8 @@ void HandleAction(v8::Isolate* isolate, void* data, string notify_type) {
break;
}
case STOP_CPU_PROFILING: {
CpuProfilerDumpData* tmp = GetDumpData<CpuProfilerDumpData>(data);
dump_data->run_once = true;
CpuProfilerDumpData* tmp = static_cast<CpuProfilerDumpData*>(data);
CpuProfiler::StopProfiling(isolate, tmp->title, cpuprofile_filepath);
AfterDumpFile(cpuprofile_filepath, notify_type, unique_key);
action_map.erase(START_CPU_PROFILING);
Expand All @@ -225,6 +204,7 @@ void HandleAction(v8::Isolate* isolate, void* data, string notify_type) {
break;
}
case STOP_SAMPLING_HEAP_PROFILING: {
dump_data->run_once = true;
SamplingHeapProfiler::StopSamplingHeapProfiling(
isolate, sampling_heapprofile_filepath);
AfterDumpFile(sampling_heapprofile_filepath, notify_type, unique_key);
Expand All @@ -237,6 +217,7 @@ void HandleAction(v8::Isolate* isolate, void* data, string notify_type) {
break;
}
case STOP_GC_PROFILING: {
dump_data->run_once = true;
GcProfiler::StopGCProfiling(isolate);
AfterDumpFile(gcprofile_filepath, notify_type, unique_key);
action_map.erase(START_GC_PROFILING);
Expand All @@ -260,9 +241,16 @@ void HandleAction(v8::Isolate* isolate, void* data, string notify_type) {
action);
break;
}

// clear dump_data
if (dump_data->run_once) {
CLEAR_DATA;
}
return;
}

#undef CHECK_ERR
#undef CLEAR_DATA

static void WaitForProfile(uint64_t profiling_time) {
uint64_t start = uv_hrtime();
Expand Down Expand Up @@ -440,6 +428,7 @@ static json DoDumpAction(json command, DumpAction action, string prefix,
data, profiling, err); \
if (err.Fail()) { \
error(format("%s", err.GetErrMessage())); \
delete data; \
return; \
} \
success(result); \
Expand Down
1 change: 0 additions & 1 deletion src/commands/dump.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ enum DumpAction {
};

using ActionMap = std::unordered_map<int, bool>;
using RequestMap = std::unordered_map<std::string, bool>;
using ConflictMap = std::unordered_map<int, std::vector<DumpAction>>;
using DependentMap = std::unordered_map<int, DumpAction>;

Expand Down