add unused input vars check for OpWithKernel, test=develop #21169

zhiqiu · 2019-11-13T09:31:05Z

In Paddle, there are several operators that registered some variables as inputs while never use them during computation in kernel. For example, instance_norm_grad registered kernel in InstanceNormGradMaker, which is not used in InstanceNormGradKernel.
This could have two problems,

Unnecessary memory consumption, especially GPU memory. Paddle uses Garbage Collection(GC) to save GPU memory, and GC relies on reference count calculation. When variables are registered as inputs, their reference count increased, thus these unused variables' memory can not be released.
This makes the code confusing.

In order to reduce unused variables, this PR,

Count variable usage in InputVar, MultiInput, MultiInputVar , compare them with op's inputs registered, and report unused variables during operator running.
Add unused_var_check to CI job. (PR_CI_Coverage)
Keep a white list which contains the existing operators that have unused variables. These operators should be fixed in the future.

See wiki for more details.

luotao1 · 2019-11-22T10:32:10Z

paddle/scripts/paddle_build.sh

        fi

        # TODO: jiabin need to refine this part when these tests fixed on mac
+        # NOTE(cql): enable unused_var_check for MAC CI


438行可以去掉了吧。439行请写具体一点，cql用完整的github名字，不然无法对应。

…le/Paddle into dev/unused_var

sneaxiy · 2019-11-28T02:11:11Z

paddle/fluid/framework/operator.cc

 }

 const Variable* ExecutionContext::InputVar(const std::string& name) const {
+  LogVarUsageIfUnusedVarCheckEnabled(name);


Call this function with if (FLAGS_enable_unused_var_check)?

Thanks. To reduce code duplication, I will put the if statement inside LogVarUsageIfUnusedVarCheckEnabled.

sneaxiy · 2019-11-28T02:11:32Z

paddle/fluid/framework/operator.cc

 template <>
 const std::vector<const Tensor*> ExecutionContext::MultiInput<Tensor>(
    const std::string& name) const {
+  LogVarUsageIfUnusedVarCheckEnabled(name);


Call this function with if (FLAGS_enable_unused_var_check)?

Thanks. To reduce code duplication, I will put the if statement inside LogVarUsageIfUnusedVarCheckEnabled.

sneaxiy · 2019-11-28T02:11:58Z

paddle/fluid/framework/operator.h


  const std::vector<const Variable*> MultiInputVar(
      const std::string& name) const {
+    LogVarUsageIfUnusedVarCheckEnabled(name);


Same above.

Done, thanks.

sneaxiy · 2019-11-28T02:12:03Z

paddle/fluid/framework/operator.h


  template <typename T>
  const std::vector<const T*> MultiInput(const std::string& name) const {
+    LogVarUsageIfUnusedVarCheckEnabled(name);


Same above.

Done, thanks.

sneaxiy · 2019-11-28T02:12:09Z

paddle/fluid/framework/operator.h


  //! Get actual name vector for this input.
  const std::vector<std::string>& Inputs(const std::string& name) const {
+    LogVarUsageIfUnusedVarCheckEnabled(name);


Same above.

Done, thanks.

sneaxiy · 2019-11-28T02:15:35Z

paddle/fluid/framework/unused_var_check.cc

+    }
+    err_msg +=
+        "please make sure it(them) is(are) needed. If not, remove it(them) "
+        "from inputs; if yes, register NoNeedBufferVars or add the operator to "


register NoNeedBufferVars or remove them in the GradOpMaker or add the operator to white list in unused_var_check.cc?

Discussed offline. I use if to discussed failure of unused_var_check on two case,

If the input var is not need in computation, the only fix suggestion is "remove them in the inputs."

If the input var is really need in computation but failed on check (due to some reasons, like different kernels, conditional usage, or only needing dims), the fix suggestion is "register NoNeedBufferVars or add the operator to white list in unused_var_check.cc".

sneaxiy · 2019-11-28T02:15:42Z

paddle/fluid/framework/unused_var_check.cc

+        "please make sure it(them) is(are) needed. If not, remove it(them) "
+        "from inputs; if yes, register NoNeedBufferVars or add the operator to "
+        "white list in unused_var_check.cc.";
+    // VLOG(1) << err_msg;


Remove unused comments.

Done, thanks.

sneaxiy · 2019-11-28T02:16:40Z

paddle/fluid/framework/unused_var_check.h

+#include "paddle/fluid/framework/operator.h"
+
+DECLARE_bool(enable_unused_var_check);
+DECLARE_bool(use_mkldnn);


Not important: DECLARE_xxx here is not necessary in header file. You can move them to .cc.

I remove DECLARE_bool(use_mkldnn) since it is not used, and keep DECLARE_bool(enable_unused_var_check) because some .cc file will include this header file and use the FLAGS.

sneaxiy · 2019-11-28T02:17:25Z

paddle/fluid/operators/flatten_op.cc

 DECLARE_INPLACE_OP_INFERER(FlattenGradInplaceinToOut,
                           {framework::GradVarName("Out"),
                            framework::GradVarName("X")});
+DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(FlattenGradNoNeedBufferVarsInference,


Why no need Out? Maybe it is X that is not needed?

sneaxiy · 2019-11-28T02:18:02Z

paddle/fluid/operators/squeeze_op.cc

                           {framework::GradVarName("Out"),
                            framework::GradVarName("X")});
-
+DECLARE_NO_NEED_BUFFER_VARS_INFERENCE(SqueezeGradNoNeedBufferVarsInference,


Same above.

…le/Paddle into dev/unused_var

luotao1 · 2019-11-28T05:56:24Z

tools/check_api_approvals.sh

          echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n"
          check_approval 1 10721757 5442383
+      elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.cc" ];then
+          echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of operators that have unused input variables.\n"


You should have a wiki to tell developers what's the problem, and how to fix this problem, otherwise, they don't understand it, and will directly add to the write list.

luotao1

LGTM

…le/Paddle into dev/unused_var

sneaxiy

LGTM with 2 things to do in the next PR:

Remove InitThreadLocalUsedVarNameSet in unused_var_check.h since it is not used and not defined.
Add custom grad op maker for squeeze and flatten to remove Out dependency.

…le/Paddle into dev/unused_var

sneaxiy

Excellent!

luotao1

LGTM

…dle#21169) * add unused input vars check for OpWithKernel, test=develop * remove unused vars in some ops, test=develop * fix batch_norm, test=develop * add white list, test=develop * add CI check for white list, test=develop * :ove white list to c++, test=develop * solve failure of CI, test=develop * add unittest for unused_var_check, test=develop * refine code, enable check in operator_test, test=develop * skip mkldnn, test=develop * extend white list, test=develop * refine condition of mkldnn, test=develop * fix paddle_build, test=develop * follow comments, test=develop * fix GetExpectedKernelType * add wiki ref to err_msg, test=develop * follow comment, test=develop

zhiqiu force-pushed the dev/unused_var branch 4 times, most recently from 740b587 to e9b1db9 Compare November 14, 2019 07:48

zhiqiu force-pushed the dev/unused_var branch 3 times, most recently from ebcc8a7 to 26d968e Compare November 20, 2019 14:57

zhiqiu added 5 commits November 21, 2019 15:35

add unused input vars check for OpWithKernel, test=develop

08d70cb

remove unused vars in some ops, test=develop

dc41961

fix batch_norm, test=develop

032ef2f

add white list, test=develop

7aff856

add CI check for white list, test=develop

e69b905

zhiqiu force-pushed the dev/unused_var branch 4 times, most recently from 936ea7f to 6552c8f Compare November 21, 2019 15:43

:ove white list to c++, test=develop

afc8da5

zhiqiu force-pushed the dev/unused_var branch from 6552c8f to afc8da5 Compare November 22, 2019 02:37

zhiqiu added 2 commits November 22, 2019 11:16

solve failure of CI, test=develop

1db659b

add unittest for unused_var_check, test=develop

5b1047d

zhiqiu force-pushed the dev/unused_var branch from c8fbf26 to 5b1047d Compare November 22, 2019 08:02

zhiqiu requested review from lanxianghit, luotao1 and sneaxiy November 22, 2019 09:10

luotao1 reviewed Nov 22, 2019

View reviewed changes

zhiqiu force-pushed the dev/unused_var branch 3 times, most recently from 50719e6 to 36487c8 Compare November 22, 2019 15:15

refine code, enable check in operator_test, test=develop

e97a64d

zhiqiu force-pushed the dev/unused_var branch from 36487c8 to e97a64d Compare November 23, 2019 17:14

luotao1 added the OpPorting label Nov 27, 2019

zhiqiu added 2 commits November 27, 2019 12:34

refine condition of mkldnn, test=develop

67b9f4f

test=develop, Merge branch 'develop' of https://github.com/PaddlePadd…

234dc12

…le/Paddle into dev/unused_var

zhiqiu force-pushed the dev/unused_var branch from 019b5bb to 5d6171c Compare November 27, 2019 16:17

fix paddle_build, test=develop

a180079

zhiqiu force-pushed the dev/unused_var branch from 5d6171c to a180079 Compare November 27, 2019 16:23

sneaxiy reviewed Nov 28, 2019

View reviewed changes

zhiqiu added 2 commits November 28, 2019 11:49

follow comments, test=develop

a470fbc

test=develop, Merge branch 'develop' of https://github.com/PaddlePadd…

0fe1a68

…le/Paddle into dev/unused_var

luotao1 reviewed Nov 28, 2019

View reviewed changes

zhiqiu added 2 commits November 28, 2019 15:22

fix GetExpectedKernelType

98f1509

add wiki ref to err_msg, test=develop

6ee0ab0

luotao1 previously approved these changes Nov 28, 2019

View reviewed changes

test=develop, Merge branch 'develop' of https://github.com/PaddlePadd…

301f835

…le/Paddle into dev/unused_var

zhiqiu dismissed luotao1’s stale review via 301f835 November 28, 2019 13:32

zhiqiu force-pushed the dev/unused_var branch from 25347ae to 4ead050 Compare November 28, 2019 15:31

test=develop, Merge branch 'develop' of https://github.com/PaddlePadd…

1d9bb16

…le/Paddle into dev/unused_var

zhiqiu force-pushed the dev/unused_var branch from 4ead050 to 1d9bb16 Compare November 29, 2019 02:33

sneaxiy previously approved these changes Nov 29, 2019

View reviewed changes

zhiqiu added 2 commits November 29, 2019 12:00

test=develop, Merge branch 'develop' of https://github.com/PaddlePadd…

c8dff60

…le/Paddle into dev/unused_var

follow comment, test=develop

cfc3a6e

zhiqiu dismissed sneaxiy’s stale review via cfc3a6e November 29, 2019 04:07

zhiqiu force-pushed the dev/unused_var branch from 253cc34 to cfc3a6e Compare November 29, 2019 04:07

sneaxiy approved these changes Nov 29, 2019

View reviewed changes

luotao1 approved these changes Nov 29, 2019

View reviewed changes

zhiqiu merged commit e0c9d85 into PaddlePaddle:develop Nov 29, 2019

luotao1 mentioned this pull request Dec 26, 2019

import paddle出现ImportError：dlopen：cannot load any more object with static TLS #21760

Closed

zhiqiu mentioned this pull request Jan 16, 2020

Remove unused inputs for some operators #22284

Merged

Uh oh!

add unused input vars check for OpWithKernel, test=develop #21169

add unused input vars check for OpWithKernel, test=develop #21169

Uh oh!

Conversation

zhiqiu commented Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhiqiu Nov 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luotao1 left a comment

Choose a reason for hiding this comment

Uh oh!

sneaxiy left a comment

Choose a reason for hiding this comment

Uh oh!

sneaxiy left a comment

Choose a reason for hiding this comment

Uh oh!

luotao1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhiqiu commented Nov 13, 2019 •

edited

Loading

zhiqiu Nov 28, 2019 •

edited

Loading