Skip to content

Commit 20c47ad

Browse files
authored
Merge pull request #127 from bwrsandman/enable-check-profile
Add a run time profile report
2 parents 0eccb1c + 44ec283 commit 20c47ad

File tree

3 files changed

+1228
-13
lines changed

3 files changed

+1228
-13
lines changed

post/clang_tidy_review/clang_tidy_review/__init__.py

+141-7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import argparse
77
import base64
88
import fnmatch
9+
import glob
910
import itertools
1011
import json
1112
import os
@@ -21,18 +22,20 @@
2122
import datetime
2223
import re
2324
import io
25+
import textwrap
2426
import zipfile
2527
from github import Github, Auth
2628
from github.GithubException import GithubException
2729
from github.Requester import Requester
2830
from github.PaginatedList import PaginatedList
2931
from github.WorkflowRun import WorkflowRun
30-
from typing import Any, List, Optional, TypedDict
32+
from typing import Any, List, Optional, TypedDict, Dict
3133

3234
DIFF_HEADER_LINE_LENGTH = 5
3335
FIXES_FILE = "clang_tidy_review.yaml"
3436
METADATA_FILE = "clang-tidy-review-metadata.json"
3537
REVIEW_FILE = "clang-tidy-review-output.json"
38+
PROFILE_DIR = "clang-tidy-review-profile"
3639
MAX_ANNOTATIONS = 10
3740

3841

@@ -175,6 +178,8 @@ def build_clang_tidy_warnings(
175178
f"-p={build_dir}",
176179
f"-line-filter={line_filter}",
177180
f"--export-fixes={FIXES_FILE}",
181+
"--enable-check-profile",
182+
f"-store-check-profile={PROFILE_DIR}",
178183
]
179184

180185
if config:
@@ -185,7 +190,6 @@ def build_clang_tidy_warnings(
185190

186191
args += files
187192

188-
start = datetime.datetime.now()
189193
try:
190194
with message_group(f"Running:\n\t{args}"):
191195
env = dict(os.environ)
@@ -201,9 +205,6 @@ def build_clang_tidy_warnings(
201205
print(
202206
f"\n\nclang-tidy failed with return code {e.returncode} and error:\n{e.stderr}\nOutput was:\n{e.stdout}"
203207
)
204-
end = datetime.datetime.now()
205-
206-
print(f"Took: {end - start}")
207208

208209

209210
def clang_tidy_version(clang_tidy_binary: pathlib.Path):
@@ -286,6 +287,13 @@ def pull_request(self):
286287
self._pull_request = self.repo.get_pull(int(self.pr_number))
287288
return self._pull_request
288289

290+
@property
291+
def head_sha(self):
292+
if self._pull_request is None:
293+
raise RuntimeError("Missing PR")
294+
295+
return self._pull_request.get_commits().reversed[0].sha
296+
289297
def get_pr_diff(self) -> List[unidiff.PatchedFile]:
290298
"""Download the PR diff, return a list of PatchedFile"""
291299

@@ -816,6 +824,86 @@ def create_review_file(
816824
return review
817825

818826

827+
def make_timing_summary(clang_tidy_profiling: Dict, sha: Optional[str] = None) -> str:
828+
if not clang_tidy_profiling:
829+
return ""
830+
top_amount = 10
831+
wall_key = "time.clang-tidy.total.wall"
832+
user_key = "time.clang-tidy.total.user"
833+
sys_key = "time.clang-tidy.total.sys"
834+
total_wall = sum(timings[wall_key] for timings in clang_tidy_profiling.values())
835+
total_user = sum(timings[user_key] for timings in clang_tidy_profiling.values())
836+
total_sys = sum(timings[sys_key] for timings in clang_tidy_profiling.values())
837+
print(f"Took: {total_user:.2f}s user {total_sys:.2f} system {total_wall:.2f} total")
838+
file_summary = textwrap.dedent(
839+
f"""\
840+
### Top {top_amount} files
841+
| File | user (s) | system (s) | total (s) |
842+
| ----- | ---------------- | --------------- | ---------------- |
843+
| Total | {total_user:.2f} | {total_sys:.2f} | {total_wall:.2f} |
844+
"""
845+
)
846+
topfiles = sorted(
847+
(
848+
(
849+
os.path.relpath(file),
850+
timings[user_key],
851+
timings[sys_key],
852+
timings[wall_key],
853+
)
854+
for file, timings in clang_tidy_profiling.items()
855+
),
856+
key=lambda x: x[3],
857+
reverse=True,
858+
)
859+
860+
if "GITHUB_SERVER_URL" in os.environ and "GITHUB_REPOSITORY" in os.environ:
861+
blob = f"{os.environ['GITHUB_SERVER_URL']}/{os.environ['GITHUB_REPOSITORY']}/blob/{sha}"
862+
else:
863+
blob = None
864+
for f, u, s, w in list(topfiles)[:top_amount]:
865+
if blob is not None:
866+
f = f"[{f}]({blob}/{f})"
867+
file_summary += f"|{f}|{u:.2f}|{s:.2f}|{w:.2f}|\n"
868+
869+
check_timings = {}
870+
for timings in clang_tidy_profiling.values():
871+
for check, timing in timings.items():
872+
if check in [wall_key, user_key, sys_key]:
873+
continue
874+
base_check, time_type = check.rsplit(".", 1)
875+
check_name = base_check.split(".", 2)[2]
876+
t = check_timings.get(check_name, (0.0, 0.0, 0.0))
877+
if time_type == "user":
878+
t = t[0] + timing, t[1], t[2]
879+
elif time_type == "sys":
880+
t = t[0], t[1] + timing, t[2]
881+
elif time_type == "wall":
882+
t = t[0], t[1], t[2] + timing
883+
check_timings[check_name] = t
884+
885+
check_summary = ""
886+
if check_timings:
887+
check_summary = textwrap.dedent(
888+
f"""\
889+
### Top {top_amount} checks
890+
| Check | user (s) | system (s) | total (s) |
891+
| ----- | -------- | ---------- | --------- |
892+
| Total | {total_user:.2f} | {total_sys:.2f} | {total_wall:.2f} |
893+
"""
894+
)
895+
topchecks = sorted(
896+
((check_name, *timings) for check_name, timings in check_timings.items()),
897+
key=lambda x: x[3],
898+
reverse=True,
899+
)
900+
for c, u, s, w in list(topchecks)[:top_amount]:
901+
c = decorate_check_names(f"[{c}]").replace("[[", "[").rstrip("]")
902+
check_summary += f"|{c}|{u:.2f}|{s:.2f}|{w:.2f}|\n"
903+
904+
return f"## Timing\n{file_summary}{check_summary}"
905+
906+
819907
def filter_files(diff, include: List[str], exclude: List[str]) -> List:
820908
changed_files = [filename.target_file[2:] for filename in diff]
821909
files = []
@@ -895,6 +983,18 @@ def create_review(
895983
# Read and parse the CLANG_TIDY_FIXES file
896984
clang_tidy_warnings = load_clang_tidy_warnings()
897985

986+
# Read and parse the timing data
987+
clang_tidy_profiling = load_and_merge_profiling()
988+
989+
try:
990+
sha = pull_request.head_sha
991+
except Exception:
992+
sha = os.environ.get("GITHUB_SHA")
993+
994+
# Post to the action job summary
995+
step_summary = make_timing_summary(clang_tidy_profiling, sha)
996+
set_summary(step_summary)
997+
898998
print("clang-tidy had the following warnings:\n", clang_tidy_warnings, flush=True)
899999

9001000
diff_lookup = make_file_line_lookup(diff)
@@ -981,6 +1081,29 @@ def load_review(review_file: pathlib.Path) -> Optional[PRReview]:
9811081
return payload or None
9821082

9831083

1084+
def load_and_merge_profiling() -> Dict:
1085+
result = dict()
1086+
for profile_file in glob.iglob(os.path.join(PROFILE_DIR, "*.json")):
1087+
profile_dict = json.load(open(profile_file))
1088+
filename = profile_dict["file"]
1089+
current_profile = result.get(filename, dict())
1090+
for check, timing in profile_dict["profile"].items():
1091+
current_profile[check] = current_profile.get(check, 0.0) + timing
1092+
result[filename] = current_profile
1093+
for filename, timings in list(result.items()):
1094+
timings["time.clang-tidy.total.wall"] = sum(
1095+
v for k, v in timings.items() if k.endswith("wall")
1096+
)
1097+
timings["time.clang-tidy.total.user"] = sum(
1098+
v for k, v in timings.items() if k.endswith("user")
1099+
)
1100+
timings["time.clang-tidy.total.sys"] = sum(
1101+
v for k, v in timings.items() if k.endswith("sys")
1102+
)
1103+
result[filename] = timings
1104+
return result
1105+
1106+
9841107
def load_and_merge_reviews(review_files: List[pathlib.Path]) -> Optional[PRReview]:
9851108
reviews = []
9861109
for file in review_files:
@@ -1088,7 +1211,18 @@ def set_output(key: str, val: str) -> bool:
10881211
return True
10891212

10901213

1091-
def decorate_comment_body(comment: str) -> str:
1214+
def set_summary(val: str) -> bool:
1215+
if "GITHUB_STEP_SUMMARY" not in os.environ:
1216+
return False
1217+
1218+
# append key-val pair to file
1219+
with open(os.environ["GITHUB_STEP_SUMMARY"], "a") as f:
1220+
f.write(val)
1221+
1222+
return True
1223+
1224+
1225+
def decorate_check_names(comment: str) -> str:
10921226
"""
10931227
Split on first dash into two groups of string in [] at end of line
10941228
exception: if the first group starts with 'clang' such as 'clang-diagnostic-error'
@@ -1102,7 +1236,7 @@ def decorate_comment_body(comment: str) -> str:
11021236

11031237

11041238
def decorate_comment(comment: PRReviewComment) -> PRReviewComment:
1105-
comment["body"] = decorate_comment_body(comment["body"])
1239+
comment["body"] = decorate_check_names(comment["body"])
11061240
return comment
11071241

11081242

0 commit comments

Comments
 (0)