tools/test: adding tools for splitting up mock headers by foreseeable · Pull Request #12150 · envoyproxy/envoy

foreseeable · 2020-07-17T14:22:48Z

Signed-off-by: Muge Chen mugechen@google.com

Commit Message: adding tools for splitting up mock headers.
Risk level: low
Testing: Build succeeds.

We need to reduce resource consumption of test compilation by simplifying mock library inclusions. #10917
One way to do that is to break the monolithic mock headers into different mock classes and then refactor the code base (like #12053 #12051 #11797)
It's hard to refactor them manually. So I wrote headersplit, a tool based on libclang and python to help me divide the monolithic mock header and replace includes after dividing automatically. This tool can also generate building time comparison between before/after refactoring.

Signed-off-by: Muge Chen <mugechen@google.com>

foreseeable · 2020-07-17T14:24:13Z

\cc @ahedberg @bbarenblat

ahedberg

One of us will do a more in-depth review later, but here are some high-level comments to get started.

Please also fix the formatting CI.

tools/envoy_headersplit/README.md

tools/envoy_headersplit/changed.txt

tools/envoy_headersplit/replace_includes.py

Signed-off-by: Muge Chen <mugechen@google.com>

tools/envoy_headersplit/README.md

tools/envoy_headersplit/headersplit.py

tools/envoy_headersplit/headersplit_test.py

tools/envoy_headersplit/replace_includes.py

tools/envoy_headersplit/replace_includes_test.py

Signed-off-by: Muge Chen <mugechen@google.com>

tools/envoy_headersplit/code_corpus/fail_mocks.h

tools/envoy_headersplit/headersplit.py

tools/envoy_headersplit/headersplit_test.py

tools/envoy_headersplit/replace_includes.py

Signed-off-by: Muge Chen <mugechen@google.com>

foreseeable · 2020-08-24T14:55:25Z

\cc @htuch

htuch

Thanks, I like where you're going with trying to make things more hermetic and nice work on getting tests happening. Some feeedback to hopefully get this to a final state.
/wait

htuch · 2020-08-24T17:38:01Z

bazel/repositories_extra.bzl

        name = "protodoc_pip3",
        requirements = "@envoy//tools/protodoc:requirements.txt",
    )
+    pip3_import(


Just to confirm the pip3 trick is so you can bring in Clang as a Python dependency during test via requirements.txt? @lizan @PiotrSikora is there going to be some Wasm dependency that eventually gives us this more directly? I think what is here right now is fine for now though.

yes I have to install the clang module so that I can use them at test.

tools/envoy_headersplit/requirements.txt

ci/do_ci.sh

htuch · 2020-08-24T17:43:32Z

tools/envoy_headersplit/BUILD

+py_test(
+    name = "headersplit_test",
+    srcs = [
+        "headersplit.py",


Shouldn't you depend on headersplit_pip3 and then not bring in the file directly?

tools/envoy_headersplit/headersplit.py

htuch · 2020-08-24T17:51:54Z

tools/envoy_headersplit/headersplit.py

+)
+""".format(to_filename(class_name), to_filename(class_name), to_filename(class_name))
+  with open("BUILD", "a") as bazel_file:
+    bazel_file.write(bazel_text)


If this is append, this script is somewhat non-idempotent. What happens if you run it multiple times?

it will add rule for same mock class target for multiple times.
Now I add statements to check if the target has been already appended. So now it will only add once if run multiple times

htuch · 2020-08-24T17:54:20Z

tools/envoy_headersplit/README.md

@@ -0,0 +1,12 @@
+# Envoy Header Split
+Tool for spliting monolithic header files in Envoy to speed up compilation


How often do you think Envoy developers should be running this? Is this a once a year kind of cleanup?

I think the period could be several months or a year. It should be long enough until we get some monolithic mock headers appeared in Envoy repository.

Signed-off-by: Muge Chen <mugechen@google.com>

foreseeable · 2020-08-26T20:34:00Z

@htuch can you have a look again?
I fixed some style issue. llvm-config is no longer needed to run the scripts since clang is installed from pip3. (But we still need to run the test un-sandboxed)

htuch

LGTM, let's add the TODO and ship it. Awesome work!

htuch · 2020-08-27T23:35:40Z

tools/envoy_headersplit/BUILD

+    main = "headersplit_test.py",
+    python_version = "PY3",
+    srcs_version = "PY3",
+    tags = ["no-sandbox"],


Can you add some TODOs to dig into this? I'm still unclear on the interaction with --spawn_strategy=local.

Signed-off-by: Muge Chen <mugechen@google.com>

htuch

LGTM, thanks!

antoniovicente · 2020-08-31T16:18:50Z

ci/run_clang_tidy.sh

 }

+# Do not run clang-tidy on envoy_headersplit testdata files.
+function exclude_testdata() {


I think that you redefined the exclude_testdata function and effectively undid the tools/testdata/check_format exclusion a few lines up.

Nice catch! Thanks for fix it

) We need to reduce resource consumption of test compilation by simplifying mock library inclusions. envoyproxy#10917 One way to do that is to break the monolithic mock headers into different mock classes and then refactor the code base (like envoyproxy#12053 envoyproxy#12051 envoyproxy#11797) It's hard to refactor them manually. So I wrote headersplit, a tool based on libclang and python to help me divide the monolithic mock header and replace includes after dividing automatically. This tool can also generate building time comparison between before/after refactoring. Risk level: low Testing: Build succeeds. Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>

antoniovicente · 2021-01-09T04:04:05Z

tools/envoy_headersplit/headersplit_test.py

@@ -0,0 +1,103 @@
+# Lint as: python3


I'm having trouble getting this test to run locally or in CI. See https://dev.azure.com/cncf/4684fb3d-0389-4e0b-8251-221942316e06/_apis/build/builds/63187/logs/48 from PR #14615 which tries to add //tools/... to CI

Do you know what may be going wrong?

====================================================================== ERROR: test_class_definitions (__main__.HeadersplitTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/headersplit_pip3_pypi__clang_10_0_1/clang/cindex.py", line 4172, in get_cindex_library library = cdll.LoadLibrary(self.get_filename()) File "/usr/lib/python3.6/ctypes/__init__.py", line 426, in LoadLibrary return self._dlltype(name) File "/usr/lib/python3.6/ctypes/__init__.py", line 348, in __init__ self._handle = _dlopen(self._name, mode) OSError: libclang-10.so: cannot open shared object file: No such file or directory During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/envoy/tools/envoy_headersplit/headersplit_test.py", line 55, in test_class_definitions idx = Index.create() File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/headersplit_pip3_pypi__clang_10_0_1/clang/cindex.py", line 2698, in create return Index(conf.lib.clang_createIndex(excludeDecls, 0)) File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/headersplit_pip3_pypi__clang_10_0_1/clang/cindex.py", line 212, in __get__ value = self.wrapped(instance) File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/headersplit_pip3_pypi__clang_10_0_1/clang/cindex.py", line 4146, in lib lib = self.get_cindex_library() File "/b/f/w/bazel-out/k8-opt/bin/tools/envoy_headersplit/headersplit_test.runfiles/headersplit_pip3_pypi__clang_10_0_1/clang/cindex.py", line 4177, in get_cindex_library raise LibclangError(msg) clang.cindex.LibclangError: libclang-10.so: cannot open shared object file: No such file or directory. To provide a path to libclang use Config.set_library_path() or Config.set_library_file().

adding tools to dividing mock headers

27876b3

Signed-off-by: Muge Chen <mugechen@google.com>

ahedberg suggested changes Jul 17, 2020

View reviewed changes

foreseeable added 2 commits July 17, 2020 15:28

fix spelling

f33ed82

Signed-off-by: Muge Chen <mugechen@google.com>

fix format issues

9579352

Signed-off-by: Muge Chen <mugechen@google.com>

junr03 assigned ahedberg Jul 17, 2020

foreseeable added 6 commits July 22, 2020 14:32

fix a minor bug in scripts

4106ddc

Signed-off-by: Muge Chen <mugechen@google.com>

adding support to binary size tracking

d13fc8f

Signed-off-by: Muge Chen <mugechen@google.com>

refactor profile.py

270933d

Signed-off-by: Muge Chen <mugechen@google.com>

added type hint and unit test

364d7b2

Signed-off-by: Muge Chen <mugechen@google.com>

update Readme

b812242

Signed-off-by: Muge Chen <mugechen@google.com>

run formatter

900eb5e

Signed-off-by: Muge Chen <mugechen@google.com>

ahedberg suggested changes Jul 28, 2020

View reviewed changes

foreseeable added 9 commits July 30, 2020 01:33

code refactoring

89c47dd

Signed-off-by: Muge Chen <mugechen@google.com>

adding comments

6b94843

Signed-off-by: Muge Chen <mugechen@google.com>

run formatter

03f5b7a

Signed-off-by: Muge Chen <mugechen@google.com>

add no lint for code corpus to pass ci

25ba2d3

Signed-off-by: Muge Chen <mugechen@google.com>

run formatter

e31448a

Signed-off-by: Muge Chen <mugechen@google.com>

format

e3d5220

Signed-off-by: Muge Chen <mugechen@google.com>

adding supports to produce changed.txt automatically

688e47c

Signed-off-by: Muge Chen <mugechen@google.com>

run formatter

810cc3f

Signed-off-by: Muge Chen <mugechen@google.com>

silence clang_tidy on test data

8e21940

Signed-off-by: Muge Chen <mugechen@google.com>

ahedberg reviewed Aug 4, 2020

View reviewed changes

tools/envoy_headersplit/code_corpus/fail_mocks.h Show resolved Hide resolved

ahedberg reviewed Aug 4, 2020

View reviewed changes

tools/envoy_headersplit/headersplit.py Outdated Show resolved Hide resolved

ahedberg suggested changes Aug 4, 2020

View reviewed changes

foreseeable added 5 commits August 6, 2020 03:15

add comments for get_headers

e5dc56a

Signed-off-by: Muge Chen <mugechen@google.com>

rename function

98d28dc

Signed-off-by: Muge Chen <mugechen@google.com>

adding docs

29f73b6

Signed-off-by: Muge Chen <mugechen@google.com>

run formatter

45c39a0

Signed-off-by: Muge Chen <mugechen@google.com>

use smaller test

9c6c7dd

Signed-off-by: Muge Chen <mugechen@google.com>

foreseeable added 6 commits August 20, 2020 08:18

format fix

6040230

Signed-off-by: Muge Chen <mugechen@google.com>

integrate unit test under bazel

4278a4c

Signed-off-by: Muge Chen <mugechen@google.com>

make test run under bazel

1bba190

Signed-off-by: Muge Chen <mugechen@google.com>

Merge remote-tracking branch 'upstream/master' into headsplit_tool

0f7ca0f

Signed-off-by: Muge Chen <mugechen@google.com>

integrate ci

891f8b3

Signed-off-by: Muge Chen <mugechen@google.com>

fix format

66be1ac

Signed-off-by: Muge Chen <mugechen@google.com>

htuch suggested changes Aug 24, 2020

View reviewed changes

repokitteh-read-only bot added the waiting label Aug 24, 2020

fix column limit

470f3c8

Signed-off-by: Muge Chen <mugechen@google.com>

repokitteh-read-only bot removed the waiting label Aug 24, 2020

foreseeable added 5 commits August 24, 2020 22:25

format

2bc5aa2

Signed-off-by: Muge Chen <mugechen@google.com>

Kick CI

31e6111

Signed-off-by: Muge Chen <mugechen@google.com>

minor fix

1922301

Signed-off-by: Muge Chen <mugechen@google.com>

fix BUILD

85fc71c

Signed-off-by: Muge Chen <mugechen@google.com>

fix format

04da966

Signed-off-by: Muge Chen <mugechen@google.com>

foreseeable mentioned this pull request Aug 26, 2020

test: refactor header inclusion to speed up building (for test/common/http) #12775

Merged

foreseeable added 2 commits August 26, 2020 19:05

add instructions at README

28833ef

Signed-off-by: Muge Chen <mugechen@google.com>

idempotent

49d6781

Signed-off-by: Muge Chen <mugechen@google.com>

htuch reviewed Aug 27, 2020

View reviewed changes

foreseeable added 2 commits August 27, 2020 23:40

add TODO

a7b40e8

Signed-off-by: Muge Chen <mugechen@google.com>

fix format

6c5f650

Signed-off-by: Muge Chen <mugechen@google.com>

htuch approved these changes Aug 28, 2020

View reviewed changes

htuch merged commit a6cfb35 into envoyproxy:master Aug 28, 2020

antoniovicente reviewed Aug 31, 2020

View reviewed changes

antoniovicente reviewed Jan 9, 2021

View reviewed changes

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 9, 2021

		@@ -0,0 +1,12 @@
		# Envoy Header Split
		Tool for spliting monolithic header files in Envoy to speed up compilation

Conversation

foreseeable commented Jul 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foreseeable commented Jul 17, 2020

Uh oh!

ahedberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

foreseeable commented Aug 24, 2020

Uh oh!

htuch left a comment

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

foreseeable commented Aug 26, 2020

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

htuch left a comment

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!

antoniovicente Jan 9, 2021 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

foreseeable commented Jul 17, 2020 •

edited

Loading

antoniovicente Jan 9, 2021 •

edited

Loading