From 7d336c66a6088af8681f1c52da7a1ca12a139a3f Mon Sep 17 00:00:00 2001 From: taoxiangdong Date: Tue, 29 Dec 2020 21:43:50 +0800 Subject: [PATCH] Free memory before return --- ge/common/profiling/profiling_manager.cc | 6 +- ge/graph/build/model_builder.cc | 12 ++-- .../load/new_model_manager/model_manager.cc | 36 ++++++---- tests/ut/ge/CMakeLists.txt | 1 + ...el_manager_model_manager_aicpu_unittest.cc | 66 +++++++++++++++++++ 5 files changed, 101 insertions(+), 20 deletions(-) create mode 100644 tests/ut/ge/graph/load/new_model_manager_model_manager_aicpu_unittest.cc diff --git a/ge/common/profiling/profiling_manager.cc b/ge/common/profiling/profiling_manager.cc index abc4a6df..92417286 100644 --- a/ge/common/profiling/profiling_manager.cc +++ b/ge/common/profiling/profiling_manager.cc @@ -94,7 +94,7 @@ ge::Status ProfilingManager::InitFromOptions(const Options &options, MsprofGeOpt if (options.profiling_mode == "1" && !options.profiling_options.empty()) { // enable profiling by ge option if (strncpy_s(prof_conf.options, MSPROF_OPTIONS_DEF_LEN_MAX, options.profiling_options.c_str(), - MSPROF_OPTIONS_DEF_LEN_MAX - 1) != EOK) { + MSPROF_OPTIONS_DEF_LEN_MAX - 1) != EOK) { GELOGE(INTERNAL_ERROR, "copy profiling_options failed."); return INTERNAL_ERROR; } @@ -124,8 +124,8 @@ ge::Status ProfilingManager::InitFromOptions(const Options &options, MsprofGeOpt return ge::PARAM_INVALID; } - if (strncpy_s(prof_conf.jobId, MSPROF_OPTIONS_DEF_LEN_MAX, options.job_id.c_str(), - MSPROF_OPTIONS_DEF_LEN_MAX - 1) != EOK) { + if (strncpy_s(prof_conf.jobId, MSPROF_OPTIONS_DEF_LEN_MAX, options.job_id.c_str(), MSPROF_OPTIONS_DEF_LEN_MAX - 1) != + EOK) { GELOGE(INTERNAL_ERROR, "copy job_id failed."); return INTERNAL_ERROR; } diff --git a/ge/graph/build/model_builder.cc b/ge/graph/build/model_builder.cc index 77f8f237..de586275 100755 --- a/ge/graph/build/model_builder.cc +++ b/ge/graph/build/model_builder.cc @@ -805,7 +805,7 @@ Status ModelBuilder::CompileSingleOp() { } void ModelBuilder::CollectCheckAicpuAttr(const OpDescPtr &op_desc, std::set &aicpu_op_types, - std::set &aicpu_tf_op_types) { + std::set &aicpu_tf_op_types) { std::string aicpu_optype; bool has_attr_check_cpu = ge::AttrUtils::GetStr(op_desc, "needCheckCpu", aicpu_optype); std::vector tf_optypes; @@ -822,7 +822,7 @@ void ModelBuilder::CollectCheckAicpuAttr(const OpDescPtr &op_desc, std::set &aicpu_op_types, - std::set &aicpu_tf_op_types) { + std::set &aicpu_tf_op_types) { std::vector aicpu_optype_list; std::vector aicpu_tf_optype_list; if (ge::AttrUtils::GetListStr(&model, "needCheckCpu", aicpu_optype_list)) { @@ -839,10 +839,10 @@ void ModelBuilder::SetModelCheckAicpuAttr(ge::Model &model, std::setGetName().c_str(), aicpu_op_types.size(), aicpu_optype_list.size(), aicpu_tf_op_types.size(), - aicpu_tf_optype_list.size()); + "Check Aicpu op types ComputeGraph: %s aicpu_op_types: %zu, aicpu_optype_list: %zu, aicpu_tf_op_types: %zu, " + "aicpu_tf_optype_list:%zu.", + compute_graph_->GetName().c_str(), aicpu_op_types.size(), aicpu_optype_list.size(), aicpu_tf_op_types.size(), + aicpu_tf_optype_list.size()); GE_CHK_BOOL_EXEC(ge::AttrUtils::SetListStr(&model, "needCheckCpu", aicpu_optype_list), return, "Set attr needCheckCpu fail."); diff --git a/ge/graph/load/new_model_manager/model_manager.cc b/ge/graph/load/new_model_manager/model_manager.cc index 4b0dbee0..01075255 100755 --- a/ge/graph/load/new_model_manager/model_manager.cc +++ b/ge/graph/load/new_model_manager/model_manager.cc @@ -1563,6 +1563,11 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op size_t aicpu_op_nums = aicpu_optype_list.size(); size_t tf_op_nums = aicpu_tf_optype_list.size(); size_t op_nums = aicpu_op_nums + tf_op_nums; + std::function callback = [&]() { + for (auto mem : allocated_mem) { + GE_CHK_RT(rtFree(mem)); + } + }; // malloc sysOpInfoList in SysOpCheckInfo status = rtMalloc(&d_req_op_list, op_nums * sizeof(SysOpInfo), RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { @@ -1575,6 +1580,7 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op status = rtMalloc(&d_res_op_list, op_nums * sizeof(SysOpInfo), RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); return RT_ERROR_TO_GE_STATUS(status); } allocated_mem.push_back(d_res_op_list); @@ -1583,6 +1589,7 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op status = rtMalloc(&d_ret_code_list, op_nums * sizeof(ReturnCode), RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); return RT_ERROR_TO_GE_STATUS(status); } allocated_mem.push_back(d_ret_code_list); @@ -1594,6 +1601,7 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op status = rtMalloc(&d_op_type_name, op_type.length(), RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); return RT_ERROR_TO_GE_STATUS(status); } allocated_mem.push_back(d_op_type_name); @@ -1611,6 +1619,7 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op status = rtMalloc(&d_op_type_name, op_type.size(), RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); return RT_ERROR_TO_GE_STATUS(status); } allocated_mem.push_back(d_op_type_name); @@ -1639,37 +1648,39 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op status = rtMalloc(&args, args_size, RT_MEMORY_HBM); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); return RT_ERROR_TO_GE_STATUS(status); } allocated_mem.push_back(args); - GE_CHK_RT( - rtMemcpy(args, sizeof(SysOpCheckInfo), reinterpret_cast(&op_check_info_req), sizeof(SysOpCheckInfo), RT_MEMCPY_HOST_TO_DEVICE)); - GE_CHK_RT(rtMemcpy(reinterpret_cast(static_cast(static_cast(reinterpret_cast(args)) + op_check_info_req.offSetLen)), - sizeof(SysOpCheckResp), reinterpret_cast(&op_check_info_res), sizeof(SysOpCheckResp), RT_MEMCPY_HOST_TO_DEVICE)); + GE_CHK_RT(rtMemcpy(args, sizeof(SysOpCheckInfo), reinterpret_cast(&op_check_info_req), sizeof(SysOpCheckInfo), + RT_MEMCPY_HOST_TO_DEVICE)); + GE_CHK_RT(rtMemcpy( + reinterpret_cast(static_cast(static_cast(reinterpret_cast(args)) + + op_check_info_req.offSetLen)), sizeof(SysOpCheckResp), reinterpret_cast(&op_check_info_res), + sizeof(SysOpCheckResp), RT_MEMCPY_HOST_TO_DEVICE)); GE_CHK_RT(rtStreamCreate(&stream, 0)); GE_CHK_RT(rtCpuKernelLaunch(nullptr, kernel_name.c_str(), 1, args, args_size, nullptr, stream)); status = rtStreamSynchronize(stream); if (status != RT_ERROR_NONE) { GELOGE(RT_FAILED, "Call rt stream sync failed, status: 0x%x", status); + GE_MAKE_GUARD(release, callback); + GE_CHK_RT(rtStreamDestroy(stream)); return RT_ERROR_TO_GE_STATUS(status); } // Check the response - SysOpCheckResp *d_op_check_info_res = reinterpret_cast(reinterpret_cast(static_cast(static_cast(reinterpret_cast(args)) + op_check_info_req.offSetLen))); + SysOpCheckResp *d_op_check_info_res = + reinterpret_cast(reinterpret_cast(static_cast(static_cast( + reinterpret_cast(args)) + op_check_info_req.offSetLen))); (void)memset_s(&op_check_info_res, sizeof(SysOpCheckResp), 0, sizeof(SysOpCheckResp)); GE_CHK_RT(rtMemcpy(&op_check_info_res, sizeof(SysOpCheckResp), d_op_check_info_res, sizeof(SysOpCheckResp), RT_MEMCPY_DEVICE_TO_HOST)); - std::function callback = [&]() { - for (auto mem : allocated_mem) { - GE_CHK_RT(rtFree(mem)); - } - GE_CHK_RT(rtStreamDestroy(stream)); - }; if (op_check_info_res.isWithoutJson) { GELOGI("No need to check aicpu in this scenoria."); GE_MAKE_GUARD(release, callback); + GE_CHK_RT(rtStreamDestroy(stream)); return SUCCESS; } uint64_t res_op_nums = op_check_info_res.opListNum; @@ -1688,6 +1699,7 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op if (res_ret_code_list.size() != res_aicpu_op_info_list.size() || res_ret_code_list.size() != res_op_nums) { GELOGE(FAILED, "Number of retcode is not equal to number of op type."); GE_MAKE_GUARD(release, callback); + GE_CHK_RT(rtStreamDestroy(stream)); return FAILED; } std::string fail_reason; @@ -1711,10 +1723,12 @@ Status ModelManager::LaunchKernelCheckAicpuOp(std::vector &aicpu_op fail_reason += "not support."; GELOGE(FAILED, "Check aicpu op_type failed. details: %s", fail_reason.c_str()); GE_MAKE_GUARD(release, callback); + GE_CHK_RT(rtStreamDestroy(stream)); return FAILED; } GE_MAKE_GUARD(release, callback); + GE_CHK_RT(rtStreamDestroy(stream)); GELOGI("Cpu kernel launch check optype task success."); return SUCCESS; } diff --git a/tests/ut/ge/CMakeLists.txt b/tests/ut/ge/CMakeLists.txt index 175774bb..fbeb9867 100755 --- a/tests/ut/ge/CMakeLists.txt +++ b/tests/ut/ge/CMakeLists.txt @@ -562,6 +562,7 @@ set(DISTINCT_GRAPH_LOAD_TEST_FILES #"graph/load/new_model_manager_davinci_model_unittest.cc" #"graph/load/new_model_manager_model_manager_unittest.cc" #"graph/load/new_model_manager_task_build_unittest.cc" + "graph/load/new_model_manager_model_manager_aicpu_unittest.cc" "graph/load/end_graph_task_unittest.cc" "graph/load/new_model_manager_event_manager_unittest.cc" #"graph/load/output_net_output_unittest.cc" diff --git a/tests/ut/ge/graph/load/new_model_manager_model_manager_aicpu_unittest.cc b/tests/ut/ge/graph/load/new_model_manager_model_manager_aicpu_unittest.cc new file mode 100644 index 00000000..0539bcdb --- /dev/null +++ b/tests/ut/ge/graph/load/new_model_manager_model_manager_aicpu_unittest.cc @@ -0,0 +1,66 @@ +/** + * Copyright 2019-2020 Huawei Technologies Co., Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include "common/debug/log.h" +#include "common/model_parser/base.h" +#include "common/properties_manager.h" +#include "common/types.h" +#include "common/l2_cache_optimize.h" + +#define private public +#define protected public +#include "graph/load/new_model_manager/model_manager.h" + +#include "common/helper/om_file_helper.h" +#include "common/op/ge_op_utils.h" +#include "graph/load/graph_loader.h" +#include "graph/load/new_model_manager/davinci_model.h" +#include "graph/load/new_model_manager/davinci_model_parser.h" +//#include "new_op_test_utils.h" +#undef private +#undef protected + +using namespace std; +using namespace testing; + +namespace ge { + +const static std::string ENC_KEY = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +class UtestModelManagerModelManagerAicpu : public testing::Test { + protected: + void SetUp() {} + + void TearDown() {} +}; + +TEST_F(UtestModelManagerModelManagerAicpu, checkAicpuOptype) { + ModelManager model_manager; + uint32_t model_id = 0; + std::vector aicpu_op_list; + std::vector aicpu_tf_list; + aicpu_tf_list.emplace_back("FrameworkOp"); + aicpu_tf_list.emplace_back("Unique"); + + model_manager.LaunchKernelCheckAicpuOp(aicpu_op_list, aicpu_tf_list); + // Load allow listener is null + //EXPECT_EQ(ge::FAILED, mm.LoadModelOffline(model_id, data, nullptr, nullptr)); +} + +} // namespace ge