From c22185d586a64f1047c84f1ca86161dbd36ddbac Mon Sep 17 00:00:00 2001 From: limingqi107 Date: Wed, 26 May 2021 11:39:49 +0800 Subject: [PATCH] fix codecheck and pclint --- .../ccsrc/backend/optimizer/common/helper.cc | 2 +- .../gpu/insert_format_transform_op.cc | 20 ++++---- mindspore/ccsrc/pipeline/jit/action.cc | 2 +- .../ps/ps_cache/ascend/ascend_ps_cache.cc | 48 +++++++++++-------- .../ccsrc/ps/ps_cache/embedding_hash_map.cc | 2 +- .../ccsrc/ps/ps_cache/ps_cache_manager.cc | 8 ++-- .../ps/ps_cache/ps_data/ps_data_prefetch.cc | 10 ++-- .../runtime/device/gpu/blocking_queue.cc | 4 +- .../device/gpu/gpu_memory_allocator.cc | 4 +- .../runtime/device/gpu/kernel_info_setter.cc | 4 +- .../runtime/device/gpu/kernel_info_setter.h | 1 + 11 files changed, 62 insertions(+), 43 deletions(-) diff --git a/mindspore/ccsrc/backend/optimizer/common/helper.cc b/mindspore/ccsrc/backend/optimizer/common/helper.cc index e327abf44e5..da40bdf4232 100644 --- a/mindspore/ccsrc/backend/optimizer/common/helper.cc +++ b/mindspore/ccsrc/backend/optimizer/common/helper.cc @@ -437,7 +437,7 @@ std::shared_ptr>> GetRealNodeUsedListByOu } else if (AnfAlgo::GetCNodeName(node) == prim::kPrimTupleGetItem->name()) { used_output_index = output_index; } else { - auto kernel_with_index = AnfAlgo::GetPrevNodeOutput(output_info.first, output_info.second - 1); + auto kernel_with_index = AnfAlgo::GetPrevNodeOutput(output_info.first, IntToSize(output_info.second - 1)); if (kernel_with_index.first.get() != node.get()) { MS_LOG(EXCEPTION) << "Get used node failed for op[" << AnfAlgo::GetCNodeName(node) << "]"; } diff --git a/mindspore/ccsrc/backend/optimizer/gpu/insert_format_transform_op.cc b/mindspore/ccsrc/backend/optimizer/gpu/insert_format_transform_op.cc index f79f0531d47..6fa9ac8a2d7 100644 --- a/mindspore/ccsrc/backend/optimizer/gpu/insert_format_transform_op.cc +++ b/mindspore/ccsrc/backend/optimizer/gpu/insert_format_transform_op.cc @@ -28,6 +28,8 @@ namespace mindspore { namespace opt { namespace { +const int kFakeTransposeShapeOneNum = 2; + std::vector TransposeAxis(const std::string &src_format, const std::string &dst_format) { if ((src_format == kOpFormat_NCHW) && (dst_format == kOpFormat_NHWC)) { return {0, 2, 3, 1}; @@ -43,14 +45,14 @@ std::vector TransposeAxis(const std::string &src_format, const std::str // 2. out_shape [x, y, 1, 1] // 3. out_shape [x, 1, y, 1] bool IsFakeTranspose(const std::vector &out_shape, const std::vector &transpose_perm) { - if (out_shape.size() != 4) { + if (out_shape.size() != device::gpu::kFormatTransformDimension) { MS_LOG(EXCEPTION) << "Invalid data shape, 4-D data was needed, but get " << out_shape.size() << "-D."; } std::vector perm1 = {0, 2, 3, 1}; std::vector perm2 = {0, 3, 1, 2}; auto num = std::count(out_shape.begin(), out_shape.end(), 1); if ((transpose_perm == perm1) || (transpose_perm == perm2)) { - if (num >= 2) { + if (num >= kFakeTransposeShapeOneNum) { return true; } } @@ -130,9 +132,9 @@ const AnfNodePtr InsertFormatTransformOp::Process(const FuncGraphPtr &graph, con if ((inputs_format[i] != kOpFormat_DEFAULT) && (inputs_format[i] != origin_data_format)) { auto input_node = AnfAlgo::GetInputNode(utils::cast(node), i); MS_EXCEPTION_IF_NULL(input_node); - auto transpose_perm = TransposeAxis(origin_data_format, inputs_format[i]); - auto transpose_op = InsertTransposeOp(graph, input_node, node, i, transpose_perm); - SetTransposeOpBuildInfo(kOpFormat_DEFAULT, inputs_format[i], transpose_op); + auto input_transpose_perm = TransposeAxis(origin_data_format, inputs_format[i]); + auto input_transpose_op = InsertTransposeOp(graph, input_node, node, i, input_transpose_perm); + SetTransposeOpBuildInfo(kOpFormat_DEFAULT, inputs_format[i], input_transpose_op); } } @@ -145,15 +147,15 @@ const AnfNodePtr InsertFormatTransformOp::Process(const FuncGraphPtr &graph, con for (size_t j = 0; j < used_node_list->size(); j++) { auto used_node = used_node_list->at(j).first; auto used_node_index = used_node_list->at(j).second - 1; - auto transpose_perm = TransposeAxis(outputs_format[i], origin_data_format); + auto output_transpose_perm = TransposeAxis(outputs_format[i], origin_data_format); if (AnfAlgo::GetCNodeName(used_node) == prim::kPrimTupleGetItem->name()) { MS_LOG(DEBUG) << "The used node of [" << node->fullname_with_scope() << "] is tuple item."; // The tuple item need get next used nodes again. - ProcessForTupleItem(graph, used_node, used_node_index, transpose_perm, outputs_format[i]); + ProcessForTupleItem(graph, used_node, used_node_index, output_transpose_perm, outputs_format[i]); continue; } - auto transpose_op = InsertTransposeOp(graph, node, used_node, used_node_index, transpose_perm); - SetTransposeOpBuildInfo(outputs_format[i], kOpFormat_DEFAULT, transpose_op); + auto output_transpose_op = InsertTransposeOp(graph, node, used_node, used_node_index, output_transpose_perm); + SetTransposeOpBuildInfo(outputs_format[i], kOpFormat_DEFAULT, output_transpose_op); } } } diff --git a/mindspore/ccsrc/pipeline/jit/action.cc b/mindspore/ccsrc/pipeline/jit/action.cc index 166742db2fa..7ca40bd049b 100644 --- a/mindspore/ccsrc/pipeline/jit/action.cc +++ b/mindspore/ccsrc/pipeline/jit/action.cc @@ -611,7 +611,7 @@ bool ExecuteAction(const ResourcePtr &res) { } #if (ENABLE_CPU && !_WIN32) -bool StartPSWorkerAction(const ResourcePtr &res) { +bool StartPSWorkerAction(const ResourcePtr &) { ps::Worker::GetInstance().Run(); return true; } diff --git a/mindspore/ccsrc/ps/ps_cache/ascend/ascend_ps_cache.cc b/mindspore/ccsrc/ps/ps_cache/ascend/ascend_ps_cache.cc index 5dfd80e80e2..60cc06e2b0e 100644 --- a/mindspore/ccsrc/ps/ps_cache/ascend/ascend_ps_cache.cc +++ b/mindspore/ccsrc/ps/ps_cache/ascend/ascend_ps_cache.cc @@ -214,14 +214,17 @@ bool AscendPsCache::HashSwapOut(void *hash_table_addr, void *swap_out_value_addr auto hash_swap_out_mod = std::make_shared(); MS_ERROR_IF_NULL(hash_swap_out_mod); hash_swap_out_mod->SetNodeName(kEmbeddingLookupOpName); - std::vector> input_shape; - std::vector> output_shape; + + std::vector hash_table_shape = {cache_vocab_size, embedding_size}; + std::vector swap_out_index_shape = {swap_out_size}; + std::vector offset_shape = {1}; + std::vector> input_shape = {hash_table_shape, swap_out_index_shape, offset_shape}; + + std::vector swap_out_value_shape = {swap_out_size, embedding_size}; + std::vector> output_shape = {swap_out_value_shape}; + std::vector input_type = {TypeId::kNumberTypeFloat32, TypeId::kNumberTypeInt32, TypeId::kNumberTypeInt32}; std::vector output_type = {TypeId::kNumberTypeFloat32}; - input_shape.push_back({cache_vocab_size, embedding_size}); - input_shape.push_back({swap_out_size}); - input_shape.push_back({1}); - output_shape.push_back({swap_out_size, embedding_size}); auto op_info = std::make_shared(kEmbeddingLookupOpName, input_shape, input_type, output_shape, output_type); RETURN_IF_FALSE(SetNodedefProto(op_info, hash_swap_out_mod)); @@ -230,10 +233,10 @@ bool AscendPsCache::HashSwapOut(void *hash_table_addr, void *swap_out_value_addr AddressPtrList kernel_outputs = { std::make_shared
(swap_out_value_addr, swap_out_size * embedding_size * sizeof(float))}; AddressPtrList kernel_workspaces; - kernel_inputs.push_back( + kernel_inputs.emplace_back( std::make_shared
(hash_table_addr, cache_vocab_size * embedding_size * sizeof(float))); - kernel_inputs.push_back(std::make_shared
(swap_out_index_addr, swap_out_size * sizeof(int))); - kernel_inputs.push_back(std::make_shared
(offset_addr_, sizeof(int))); + kernel_inputs.emplace_back(std::make_shared
(swap_out_index_addr, swap_out_size * sizeof(int))); + kernel_inputs.emplace_back(std::make_shared
(offset_addr_, sizeof(int))); auto ret = hash_swap_out_mod->Launch(kernel_inputs, kernel_workspaces, kernel_outputs, stream_); if (!ret) { MS_LOG(ERROR) << "Hash swap out launch failed."; @@ -250,16 +253,18 @@ bool AscendPsCache::HashSwapIn(void *hash_table_addr, void *swap_in_value_addr, auto hash_swap_in_mod = std::make_shared(); MS_ERROR_IF_NULL(hash_swap_in_mod); hash_swap_in_mod->SetNodeName(kernel::kUpdateCache); - std::vector> input_shape; - std::vector> output_shape; + + std::vector hash_table_shape = {cache_vocab_size, embedding_size}; + std::vector swap_in_index_shape = {swap_in_size}; + std::vector swap_in_value_shape = {swap_in_size, embedding_size}; + std::vector offset_shape = {1}; + std::vector> input_shape = {hash_table_shape, swap_in_index_shape, swap_in_value_shape, + offset_shape}; + std::vector> output_shape = {offset_shape}; + std::vector input_type = {TypeId::kNumberTypeFloat32, TypeId::kNumberTypeInt32, TypeId::kNumberTypeFloat32, TypeId::kNumberTypeInt32}; std::vector output_type = {TypeId::kNumberTypeInt32}; - input_shape.push_back({cache_vocab_size, embedding_size}); - input_shape.push_back({swap_in_size}); - input_shape.push_back({swap_in_size, embedding_size}); - input_shape.push_back({1}); - output_shape.push_back({1}); auto op_info = std::make_shared(kernel::kUpdateCache, input_shape, input_type, output_shape, output_type); SetNodedefProto(op_info, hash_swap_in_mod); @@ -267,13 +272,14 @@ bool AscendPsCache::HashSwapIn(void *hash_table_addr, void *swap_in_value_addr, AddressPtrList kernel_inputs; AddressPtrList kernel_outputs; AddressPtrList kernel_workspaces; - kernel_inputs.push_back( + kernel_inputs.emplace_back( std::make_shared
(hash_table_addr, cache_vocab_size * embedding_size * sizeof(float))); - kernel_inputs.push_back(std::make_shared
(swap_in_index_addr, swap_in_size * sizeof(int))); - kernel_inputs.push_back(std::make_shared
(swap_in_value_addr, swap_in_size * embedding_size * sizeof(float))); - kernel_inputs.push_back(std::make_shared
(cache_vocab_size_addr_, sizeof(int))); + kernel_inputs.emplace_back(std::make_shared
(swap_in_index_addr, swap_in_size * sizeof(int))); + kernel_inputs.emplace_back( + std::make_shared
(swap_in_value_addr, swap_in_size * embedding_size * sizeof(float))); + kernel_inputs.emplace_back(std::make_shared
(cache_vocab_size_addr_, sizeof(int))); // The output of updateCache kernel is required but not useful, so any address can be assigned. - kernel_outputs.push_back(std::make_shared
(offset_addr_, sizeof(int))); + kernel_outputs.emplace_back(std::make_shared
(offset_addr_, sizeof(int))); auto ret = hash_swap_in_mod->Launch(kernel_inputs, kernel_workspaces, kernel_outputs, stream_); if (!ret) { MS_LOG(ERROR) << "Hash swap in launch failed."; diff --git a/mindspore/ccsrc/ps/ps_cache/embedding_hash_map.cc b/mindspore/ccsrc/ps/ps_cache/embedding_hash_map.cc index 25d58b87ee6..b122d9fb1a8 100755 --- a/mindspore/ccsrc/ps/ps_cache/embedding_hash_map.cc +++ b/mindspore/ccsrc/ps/ps_cache/embedding_hash_map.cc @@ -48,7 +48,7 @@ int EmbeddingHashMap::ParseData(const int id, int *const swap_out_index, int *co return hash_index; } -int EmbeddingHashMap::FindInsertionPos(const size_t data_step, const size_t graph_running_step, bool *const need_swap, +int EmbeddingHashMap::FindInsertionPos(const size_t, const size_t graph_running_step, bool *const need_swap, bool *const need_wait_graph) { MS_EXCEPTION_IF_NULL(need_swap); MS_EXCEPTION_IF_NULL(need_wait_graph); diff --git a/mindspore/ccsrc/ps/ps_cache/ps_cache_manager.cc b/mindspore/ccsrc/ps/ps_cache/ps_cache_manager.cc index 0edce622081..c48d5c41d13 100644 --- a/mindspore/ccsrc/ps/ps_cache/ps_cache_manager.cc +++ b/mindspore/ccsrc/ps/ps_cache/ps_cache_manager.cc @@ -1087,7 +1087,7 @@ void PsCacheManager::DumpHashTables(bool dump_device_tables) const { << ", device cache address:" << reinterpret_cast(item.second.device_address.addr) << ", host cache address:" << reinterpret_cast(item.second.host_address.get()); if (dump_device_tables) { - std::unique_ptr output = std::make_unique(item.second.device_address.size / 4); + std::unique_ptr output = std::make_unique(item.second.device_address.size / sizeof(float)); embedding_device_cache_->cache_->CopyDeviceMemToHost(output.get(), item.second.device_address.addr, item.second.device_address.size); embedding_device_cache_->cache_->SynchronizeStream(); @@ -1104,6 +1104,7 @@ void PsCacheManager::DumpHashTables(bool dump_device_tables) const { void PsCacheManager::DumpStatisticsInfo(size_t each_print_step) { // Default each 1000 step prints ps cache hit rate. + const size_t kFloatToPercentSign = 100; if (data_step_ % each_print_step == 0) { statistics_info_.batch_id_unique_count_ = statistics_info_.hash_hit_count_ + statistics_info_.host_to_device_size_; auto repeat_rate = SizeToFloat(statistics_info_.batch_id_count_ - statistics_info_.batch_id_unique_count_) / @@ -1117,8 +1118,9 @@ void PsCacheManager::DumpStatisticsInfo(size_t each_print_step) { << ", device swap to host num:" << statistics_info_.device_to_host_size_ << ", host swap to server num:" << statistics_info_.host_to_server_size_ << ", server swap to host num:" << statistics_info_.server_to_host_size_ - << ", data repeat rate:" << repeat_rate * 100 << "%, device cache hit rate:" << device_hit_rate * 100 - << "%, host cache hit rate:" << host_hit_rate * 100 << ")."; + << ", data repeat rate:" << (repeat_rate * kFloatToPercentSign) + << "%, device cache hit rate:" << (device_hit_rate * kFloatToPercentSign) + << "%, host cache hit rate:" << (host_hit_rate * kFloatToPercentSign) << ")."; } } } // namespace ps diff --git a/mindspore/ccsrc/ps/ps_cache/ps_data/ps_data_prefetch.cc b/mindspore/ccsrc/ps/ps_cache/ps_data/ps_data_prefetch.cc index df96ac28feb..31e8d070c5a 100644 --- a/mindspore/ccsrc/ps/ps_cache/ps_data/ps_data_prefetch.cc +++ b/mindspore/ccsrc/ps/ps_cache/ps_data/ps_data_prefetch.cc @@ -19,6 +19,8 @@ namespace mindspore { namespace ps { +const size_t kTimeoutLoopCount = 10; + void PsDataPrefetch::CreateDataChannel(const std::string &channel_name, size_t step_num) { if (cache_enable_ == false) { return; @@ -69,7 +71,8 @@ bool PsDataPrefetch::PrefetchData(const std::string &channel_name, void *data, c if (!need_wait_) { return true; } - for (int i = 0; i < 10; i++) { + + for (size_t i = 0; i < kTimeoutLoopCount; ++i) { if (data_prefetch_.wait_for(locker, std::chrono::seconds(30), [this] { return data_ready_ == false || need_wait_ == false; })) { return true; @@ -94,7 +97,8 @@ bool PsDataPrefetch::FinalizeData(const std::string &channel_name) { if (!need_wait_) { return true; } - for (int i = 0; i < 10; i++) { + + for (size_t i = 0; i < kTimeoutLoopCount; ++i) { if (data_process_.wait_for(locker, std::chrono::seconds(30), [this] { return data_ready_ == true || need_wait_ == false; })) { return true; @@ -147,7 +151,7 @@ bool PsDataPrefetch::TryWakeChannel(const std::string &channel_name) { } void PsDataPrefetch::WakeAllChannel() { - for (auto iter = ps_data_channel_map_.begin(); iter != ps_data_channel_map_.end(); iter++) { + for (auto iter = ps_data_channel_map_.begin(); iter != ps_data_channel_map_.end(); ++iter) { auto channel = iter->second; if (channel == nullptr) { return; diff --git a/mindspore/ccsrc/runtime/device/gpu/blocking_queue.cc b/mindspore/ccsrc/runtime/device/gpu/blocking_queue.cc index ff48c6de41f..ad4d65b1814 100644 --- a/mindspore/ccsrc/runtime/device/gpu/blocking_queue.cc +++ b/mindspore/ccsrc/runtime/device/gpu/blocking_queue.cc @@ -21,6 +21,8 @@ namespace mindspore { namespace device { +const size_t kTimeout = 100; + GpuQueue::GpuQueue(void *addr, const std::vector &shape, const size_t &capacity) : buffer_(addr), head_(0), @@ -110,7 +112,7 @@ void BlockingQueue::RegisterRelease(const std::function & BlockQueueStatus_T BlockingQueue::Push(const std::vector &data, unsigned int) { std::unique_lock locker(mutex_); if (queue_->IsFull()) { - if (not_full_cond_.wait_for(locker, std::chrono::microseconds(100)) == std::cv_status::timeout) { + if (not_full_cond_.wait_for(locker, std::chrono::microseconds(kTimeout)) == std::cv_status::timeout) { return TIMEOUT; } } diff --git a/mindspore/ccsrc/runtime/device/gpu/gpu_memory_allocator.cc b/mindspore/ccsrc/runtime/device/gpu/gpu_memory_allocator.cc index 2a8bc5cc0d7..46a135ffffe 100644 --- a/mindspore/ccsrc/runtime/device/gpu/gpu_memory_allocator.cc +++ b/mindspore/ccsrc/runtime/device/gpu/gpu_memory_allocator.cc @@ -24,13 +24,15 @@ namespace mindspore { namespace device { namespace gpu { +const size_t kGBToByte = 1024 << 20; + bool GPUMemoryAllocator::Init() { size_t total_size = total_mem_size(); size_t free_size = CudaDriver::free_mem_size(); auto context_ptr = MsContext::GetInstance(); MS_EXCEPTION_IF_NULL(context_ptr); limited_device_memory_ = context_ptr->get_param(MS_CTX_MAX_DEVICE_MEMORY); - available_device_memory_ = FloatToSize(limited_device_memory_ * 1024 * 1024 * 1024); + available_device_memory_ = FloatToSize(limited_device_memory_ * kGBToByte); if (total_size > 0 && free_size > 0 && available_device_memory_ > 0) { MS_LOG(INFO) << "GPU device total memory size " << total_size << ", current free memory size " << free_size << ", set max available memory size " << available_device_memory_ << "."; diff --git a/mindspore/ccsrc/runtime/device/gpu/kernel_info_setter.cc b/mindspore/ccsrc/runtime/device/gpu/kernel_info_setter.cc index 005f38679e3..036b065d9cb 100644 --- a/mindspore/ccsrc/runtime/device/gpu/kernel_info_setter.cc +++ b/mindspore/ccsrc/runtime/device/gpu/kernel_info_setter.cc @@ -220,7 +220,7 @@ bool IsNeedProcessFormatInfo(const CNodePtr &kernel_node, const std::vector, used for getting the inserted position of format transform. // If the inserted position is kAllPositions, then insert all the positions, because the input or output numbers of