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.

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

[CodeStyle] fix cpplint hook not working on CI #46136

Merged
merged 6 commits into from
Sep 19, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Sep 16, 2022

PR types

Others

PR changes

Others

Describe

修复 cpplint hook 在 CI 上不 work 的问题(详情见 #46102),以尽快阻止增量问题,存量问题由修改相关文件触发问题的 RD 处理(๑>؂<๑)

存量问题统计

所有文件(含 cuh,不含 kps),(log 见 cpplint.log)的统计如下:

{
  "paddle/fluid/framework/fleet/heter_ps/heter_comm_kernel.cu": 69,
  "paddle/fluid/framework/fleet/heter_ps/hashtable_kernel.cu": 64,
  "paddle/fluid/framework/fleet/heter_ps/cudf/concurrent_unordered_map.cuh.h": 62,
  "paddle/fluid/distributed/ps/table/common_graph_table.cc": 60,
  "paddle/fluid/distributed/ps/service/graph_brpc_client.cc": 55,
  "paddle/fluid/distributed/ps/service/graph_brpc_server.cc": 49,
  "paddle/fluid/distributed/ps/thirdparty/round_robin.h": 45,
  "paddle/fluid/framework/fleet/heter_ps/graph_gpu_ps_table_inl.cu": 36,
  "paddle/fluid/framework/fleet/heter_ps/feature_value.h": 33,
  "paddle/fluid/framework/fleet/heter_ps/graph_gpu_wrapper.cu": 29,
  "paddle/fluid/distributed/ps/table/common_graph_table.h": 28,
  "paddle/utils/variant.h": 27,
  "paddle/fluid/distributed/ps/table/ctr_double_accessor.cc": 25,
  "paddle/fluid/framework/fleet/heter_ps/heter_comm_inl.h": 24,
  "paddle/fluid/distributed/ps/service/graph_brpc_server.h": 21,
  "paddle/utils/small_vector.h": 19,
  "paddle/fluid/framework/fleet/heter_ps/heter_comm_kernel.h": 17,
  "paddle/fluid/framework/data_feed.cu": 16,
  "paddle/fluid/distributed/ps/service/ps_local_client.h": 16,
  "paddle/fluid/distributed/ps/service/ps_local_client.cc": 14,
  "paddle/utils/flat_hash_map.h": 14,
  "paddle/fluid/framework/fleet/heter_ps/test_sample_rate.cu": 13,
  "paddle/utils/array_ref.h": 12,
  "paddle/fluid/framework/fleet/ps_gpu_wrapper.cc": 12,
  "paddle/fluid/distributed/ps/table/graph/graph_node.h": 11,
  "paddle/fluid/framework/fleet/heter_ps/feature_value.cu": 10,
  "paddle/fluid/distributed/ps/table/ctr_double_accessor.h": 10,
  "paddle/fluid/framework/fleet/heter_ps/cudf/hash_functions.cuh": 9,
  "paddle/fluid/distributed/ps/table/ssd_sparse_table.cc": 9,
  "paddle/fluid/distributed/ps/table/ssd_sparse_table.h": 9,
  "paddle/utils/optional.h": 9,
  "paddle/fluid/distributed/ps/service/graph_brpc_client.h": 8,
  "paddle/fluid/framework/device_worker.cc": 8,
  "paddle/utils/any.h": 8,
  "paddle/phi/kernels/gpu/depthwise_conv.h": 7,
  "paddle/fluid/distributed/ps/table/common_table.h": 6,
  "paddle/utils/string/string_helper.h": 6,
  "paddle/fluid/framework/fleet/heter_ps/gpu_graph_node.h": 6,
  "paddle/fluid/framework/fleet/heter_ps/test_cpu_query.cu": 6,
  "paddle/fluid/operators/jit/gen/act.h": 6,
  "paddle/fluid/framework/fleet/heter_ps/optimizer.cuh.h": 6,
  "paddle/fluid/framework/fleet/heter_ps/heter_comm.h": 6,
  "paddle/fluid/framework/data_feed.h": 5,
  "paddle/fluid/distributed/ps/service/ps_service/graph_py_service.h": 5,
  "paddle/fluid/framework/fleet/heter_ps/cudf/managed_allocator.cuh": 5,
  "paddle/phi/kernels/impl/renorm_impl.h": 5,
  "paddle/fluid/distributed/ps/service/server.h": 5,
  "paddle/fluid/framework/fleet/heter_ps/hashtable.h": 4,
  "paddle/phi/kernels/cpu/graph_sample_neighbors_kernel.cc": 4,
  "paddle/fluid/distributed/ps/table/depends/rocksdb_warpper.h": 4,
  "paddle/fluid/framework/fleet/heter_ps/mem_pool.h": 4,
  "paddle/fluid/distributed/ps/service/ps_service/graph_py_service.cc": 3,
  "paddle/fluid/framework/fleet/heter_ps/test_cpu_graph_sample.cu": 3,
  "paddle/fluid/framework/fleet/heter_ps/test_graph.cu": 3,
  "paddle/fluid/framework/fleet/heter_wrapper.h": 3,
  "paddle/fluid/framework/fleet/heter_ps/cudf/managed.cuh": 3,
  "paddle/fluid/framework/fleet/heter_ps/graph_gpu_wrapper.h": 3,
  "paddle/fluid/distributed/ps/wrapper/ps_wrapper.h": 3,
  "paddle/fluid/distributed/ps/table/graph/graph_weighted_sampler.h": 3,
  "paddle/fluid/framework/fleet/ps_gpu_wrapper.h": 3,
  "paddle/fluid/distributed/ps/wrapper/fleet.cc": 3,
  "paddle/fluid/framework/fleet/heter_ps/graph_sampler_inl.h": 3,
  "paddle/fluid/framework/ir/mkldnn/conv_activation_mkldnn_fuse_pass.h": 2,
  "paddle/fluid/distributed/common/registerer.h": 2,
  "paddle/fluid/distributed/ps/table/ctr_dymf_accessor.h": 2,
  "paddle/fluid/framework/tensor_util_test.cc": 2,
  "paddle/fluid/platform/mkldnn_helper.h": 2,
  "paddle/phi/kernels/impl/repeat_interleave_kernel_impl.h": 2,
  "paddle/fluid/distributed/test/graph_node_split_test.cc": 2,
  "paddle/fluid/framework/fleet/heter_ps/heter_ps.h": 2,
  "paddle/phi/kernels/gpu/graph_send_ue_recv_funcs.h": 2,
  "paddle/fluid/framework/fleet/heter_ps/gpu_graph_utils.h": 2,
  "paddle/phi/kernels/funcs/gpc.h": 2,
  "paddle/fluid/framework/fleet/heter_ps/graph_gpu_ps_table.h": 2,
  "paddle/fluid/framework/fleet/heter_ps/graph_sampler.h": 2,
  "paddle/fluid/operators/gather_scatter_kernel.cc": 2,
  "paddle/fluid/framework/device_worker.h": 2,
  "paddle/fluid/inference/tensorrt/plugin/gelu_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/hard_swish_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/matmul_op_int8_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/trt_plugin.cc": 1,
  "paddle/fluid/operators/mkldnn/axpy_handler.cc": 1,
  "paddle/phi/kernels/impl/graph_message_passing_impl.h": 1,
  "paddle/fluid/distributed/ps/table/ctr_dymf_accessor.cc": 1,
  "paddle/fluid/distributed/ps/table/tensor_table.h": 1,
  "paddle/fluid/distributed/test/brpc_service_sparse_sgd_test.cc": 1,
  "paddle/fluid/framework/fleet/heter_ps/heter_ps.cu": 1,
  "paddle/fluid/framework/ir/mkldnn/matmul_activation_mkldnn_fuse_pass.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/emb_eltwise_layernorm_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/layernorm_shift_partition_op.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/preln_residual_bias_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/skip_layernorm_op_plugin.h": 1,
  "paddle/fluid/operators/collective/c_sync_calc_stream_op.h": 1,
  "paddle/fluid/operators/fused/fused_transformer_op.cu": 1,
  "paddle/fluid/operators/fused/mkldnn/multi_gru_mkldnn_op.cc": 1,
  "paddle/fluid/operators/group_norm_op.cc": 1,
  "paddle/fluid/operators/pull_gpups_sparse_op.cu": 1,
  "paddle/fluid/operators/sequence_ops/sequence_erase_op.h": 1,
  "paddle/phi/core/compat/convert_utils.h": 1,
  "paddle/fluid/distributed/common/topk_calculator.h": 1,
  "paddle/fluid/distributed/ps/service/ps_local_server.h": 1,
  "paddle/fluid/distributed/ps/table/depends/sparse_utils.h": 1,
  "paddle/fluid/distributed/ps/table/graph/graph_node.cc": 1,
  "paddle/fluid/imperative/nccl_context.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/anchor_generator_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/c_allreduce_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/gather_nd_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/qkv_to_context_plugin.cu": 1,
  "paddle/fluid/inference/tensorrt/plugin/stack_op_plugin.h": 1,
  "paddle/fluid/inference/tests/api/analyzer_mmp_tester.cc": 1,
  "paddle/fluid/operators/mkldnn/dequantize_mkldnn_op.cc": 1,
  "paddle/fluid/platform/device/gpu/gpu_info.cc": 1,
  "paddle/phi/kernels/fold_kernel.h": 1,
  "paddle/fluid/distributed/collective/ProcessGroupCustom.h": 1,
  "paddle/fluid/distributed/ps/table/depends/feature_value.h": 1,
  "paddle/fluid/distributed/test/table_test.cc": 1,
  "paddle/fluid/framework/fleet/heter_ps/heter_ps.cc": 1,
  "paddle/fluid/framework/ir/mkldnn/cpu_bfloat16_pass_tester.cc": 1,
  "paddle/fluid/imperative/hccl_context.h": 1,
  "paddle/fluid/inference/experimental/javaapi/native/jni_convert_util.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/elementwise_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/pool_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/qkv_to_context_plugin.h": 1,
  "paddle/fluid/operators/mkldnn/quantize_mkldnn_op.cc": 1,
  "paddle/phi/kernels/impl/renorm_kernel_impl.h": 1,
  "paddle/phi/kernels/logcumsumexp_grad_kernel.h": 1,
  "paddle/phi/kernels/renorm_grad_kernel.h": 1,
  "paddle/fluid/distributed/collective/ProcessGroupHCCL.h": 1,
  "paddle/fluid/distributed/ps/service/ps_client.h": 1,
  "paddle/fluid/distributed/ps/table/graph/graph_weighted_sampler.cc": 1,
  "paddle/fluid/framework/ir/subgraph_detector.cc": 1,
  "paddle/fluid/inference/tensorrt/plugin/group_norm_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/recover_padding_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/slice_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/transformer_input_convert_plugin.h": 1,
  "paddle/fluid/operators/gather_scatter_kernel.cu": 1,
  "paddle/fluid/operators/mkldnn/requantize_mkldnn_op.cc": 1,
  "paddle/phi/core/ddim.h": 1,
  "paddle/fluid/distributed/ps/table/accessor.h": 1,
  "paddle/fluid/framework/fleet/fleet_wrapper.h": 1,
  "paddle/fluid/inference/tensorrt/convert/group_norm_op.cc": 1,
  "paddle/phi/kernels/fold_grad_kernel.h": 1,
  "paddle/fluid/distributed/ps/service/ps_service/service.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/layer_norm_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/prelu_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/remove_padding_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/roi_align_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/split_op_plugin.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/swish_op_plugin.h": 1,
  "paddle/fluid/operators/collective/c_sync_comm_stream_op.h": 1,
  "paddle/fluid/framework/fleet/heter_ps/test_comm.cu": 1,
  "paddle/fluid/imperative/cncl_context.h": 1,
  "paddle/fluid/inference/tensorrt/plugin/pool3d_op_plugin.h": 1,
  "paddle/fluid/operators/is_empty_op.cc": 1,
  "paddle/fluid/platform/device/gpu/gpu_info.h": 1,
  "paddle/phi/kernels/impl/logcumsumexp_grad_impl.h": 1
}
Number of files: 156
Total: 1083

可以看出主要是分布式相关(distributedfleet)的问题比较多,其余文件大多只有一两个问题,在修改相关文件时一并修复即可

仅 kps 文件,(log 见 cpplint-kps-only.log)的统计如下:

{
  "paddle/fluid/framework/fleet/heter_ps/hashtable_kernel.kps": 31,
  "paddle/fluid/framework/fleet/heter_ps/heter_comm_kernel.kps": 26,
  "paddle/fluid/framework/fleet/ps_gpu_wrapper.kps": 19
}
Number of files: 3
Total: 76

@paddle-bot
Copy link

paddle-bot bot commented Sep 16, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Sep 16, 2022

exit $TOTAL_ERRORS
cpplint $@
Copy link
Member Author

@SigureMo SigureMo Sep 16, 2022

Choose a reason for hiding this comment

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

相关参数写在 pre-commit-config.yaml 里,以便维护,本 hook 仅做安装 + 调用

@@ -47,6 +47,10 @@ repos:
entry: bash ./tools/codestyle/cpplint_pre_commit.hook
language: system
files: \.(c|cc|cxx|cpp|cu|h|hpp|hxx)$
args:
- --extensions=c,cc,cxx,cpp,cu,cuh,h,hpp,hxx,kps
Copy link
Member Author

@SigureMo SigureMo Sep 16, 2022

Choose a reason for hiding this comment

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

默认 extensions 无法处理 kps,因此将 kps 后缀添加到 --extensions 参数,不过因为现在 files 字段没有传入 kps 文件,因此还不会检测 kps 文件,如果需要检测的话,在 files 字段加入 kps 即可

args:
- --extensions=c,cc,cxx,cpp,cu,cuh,h,hpp,hxx,kps
- --filter=-readability/fn_size,-build/include_what_you_use,-build/c++11,-whitespace/parens
- --quiet
Copy link
Member Author

@SigureMo SigureMo Sep 16, 2022

Choose a reason for hiding this comment

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

cpplint 默认行为是所有传入的文件都会打印 Done processing <file_path>,使用 --quiet 参数可以避免打印 Success 了的文件信息(Failed 还会打印),使得 log 更加清晰

@@ -35,6 +35,7 @@ namespace distributed {
}

int32_t GraphBrpcServer::Initialize() {
// do some change
Copy link
Member Author

Choose a reason for hiding this comment

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

修改后可以触发 CI 上 pre-commit 的拦截,流水线见 https://xly.bce.baidu.com/paddlepaddle/paddle/newipipe/detail/6641390/job/18513668

image

@SigureMo SigureMo changed the title [CodeStyle] fix cpplint hook not work on CI [CodeStyle] fix cpplint hook not working on CI Sep 16, 2022
@luotao1 luotao1 self-assigned this Sep 19, 2022
@luotao1
Copy link
Contributor

luotao1 commented Sep 19, 2022

请解决下冲突,可以使用test=document_fix来加速CI

@SigureMo
Copy link
Member Author

SigureMo commented Sep 19, 2022

请解决下冲突,可以使用test=document_fix来加速CI

冲突已解决

Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit 9d0d4a1 into PaddlePaddle:develop Sep 19, 2022
@SigureMo SigureMo deleted the pre-commit-hook-cpplint-update branch September 19, 2022 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants