New issue
Advanced search Search tips

Issue 768948 link

Starred by 7 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

clang-format presubmit check is extremely slow

Project Member Reported by danakj@chromium.org, Sep 26 2017

Issue description

The clang-format check is taking many seconds to complete. For example on this CL https://chromium-review.googlesource.com/c/chromium/src/+/683242 it is taking 13 seconds (of the 30 seconds spent in presubmit checks).

It is needlessly slow! All we need is a true/false answer from clang-format if it would change something.

Remember that presubmit already has the diff for everything.

To do this check right now:

1. In presubmit_canned_checks.py we run git-cl format --dry-run --presubmit, and check if the exit code is 2.

2. In git_cl.py we
  a. get the upstream branch
  b. run git diff -U0
  c. string-copy and pass the entire diff output to clang-format-diff.py

3. In clang-format-diff.py we
  d. parse the line numbers from the diff (this is info we could already have from presubmit_support.py's ChangedContents()
  e. run clang-format *FOR EACH DIFF HUNK*, passing it the line numbers and file path
     - note that we already have the changed lines in memory for presubmit

4. clang-format will *for each diff hunk*
  f. load the entire file into memory
  g. modify and dump the entire file to std out

5. In clang-format-diff.py we *for each diff hunk*
  i. load the entire file into memory from disk again
  j. run diff against the entire stdout from clang-format
  k. return 0 if they match, or 2 if they don't

As you can see we're doing work that we don't need to:
- Diffing against upstream
- Loading each entire file from disk 2*(# of diffs inside the file) times
- Performing a diff against the entire file

All we acually need to do is:
- Grab the diff file numbers from presubmit_support.py
- Pass all the line number sets to clang-format in one command
  - Load the file from disk once (this is sad and it'd be nicer to make it work on the file data from presubmit_support.py. Even if it was the NewContents() since we cache that.
- Get back an exit code 0/2 from clang-format without dumping the actual diff. It can early out on the first difference.
- Return that to presubmit_canned_checks.py
 
Components: -Infra Infra>SDK
Status: WontFix (was: Untriaged)
clang-format-diff.py is not owned by infra team or even chromium team; you should speak to djasper@, the owner of clang-format and associated scripts. If it grows a more efficient mode, then we can talk about PRESUBMIT.py passing it more appropriate information.

Sign in to add a comment