Improve perfomance during `pre-commit --all` (`-a`) run

[MaxymVlasov]

Some hooks recursively check all files in provided dir.

So performance degradation exists only in the pre-commit run --all situation, because it will provide all existing repo files to hook:

pre-commit-terraform/terrascan.sh

Lines 15 to 19 in e6ffbcd

	# consume modified files passed from pre-commit so that
	# terrascan runs against only those relevant directories
	for file_with_path in "${files[@]}"; do
	file_with_path="${file_with_path// /__REPLACED__SPACE__}"
	paths[index]=$(dirname "$file_with_path")

Then, unique paths are found and run terrascan for each repo folder:

pre-commit-terraform/terrascan.sh

Lines 29 to 30 in e6ffbcd

	# for each path run terrascan
	for path_uniq in $(echo "${paths[*]}" | tr ' ' '\n' | sort -u); do

It works literally how it should work: checks only diffs.

So, good to know when the --all (-a) argument passed to pre-commit and just run terrascan -d GIT_REPO_ROOT, not all-all dirs.

---

Useful info

• How to run hook performance test

• pre-commit automatically parallel checks to exiting cores, so you need to run tests on a repo that has at least 2x more tf-dirs than CPU cores you have. If you have not so big repo - just copy-paste code-structure a few times, and you'll get needed.

• Create solution as function, that can be called from terrascan_() hook function and depends on the result, in if run other flow and stop execution with exit 0

---

• checkov run checks recursively. need -d GIT_REPO_POOT_PATH feat: Add terraform_checkov, that run per folder. Deprecate checkov hook #290

• tfsec runs recursively without any additional args. feat: Improved speed of pre-commit run -a for multiple hooks #338

• terragrunt_fmt ( terragrunt hclfmt) runs recursively without any additional args. feat: Improved speed of pre-commit run -a for multiple hooks #338

• terrascan runs recursively without any additional args. feat: terrascan - Improve performance during pre-commit --all (-a) run #327

• terragrunt_validate have option terragrunt run-all validate that run checks recursively. feat: Improved speed of pre-commit run -a for multiple hooks #338

---

Founded in #305

[MaxymVlasov]

In #290 (checkov) we see the same issue.

[carlosbustillordguez]

For terrascan run the recursive scan can be disabled by passing the --non-recursive, which does not scan directories and modules recursively. Can be defined as an argument in the hook settings:

- id: terrascan
  args:
    - --args=--non-recursive # avoids scan errors on subdirectories without Terraform config files
    - --args=--policy-type=azure

I noted that pre-commit try-repo ignores the .pre-commit-config.yaml config file, so the recursive scan (if what we want) could be disabled in the hook settings:

pre-commit-terraform/.pre-commit-hooks.yaml

Line 110 in ac9299c

entry: terrascan.sh

by adding --args=--non-recursive argument at the end of the line.

For testing purposes (meantime we found a solution to detect when --all | -a was passed in pre-commit), I think we can use the --files argument (pre-commit try-repo supports all available options for pre-commit run).

For example:

TEST_NUM=5
TEST_DIR='/home/carlos/devel/azure/terraform/test/terraform_big_repo'
TEST_DESCRIPTION='`terrascan` for big TF repo with pre-commit-terraform v1.62.3'
RAW_TEST_RESULTS_FILE_NAME='terrascan_big_terrfaform_repo'

# if you don't have a .tf file in the root directory, run the following:
touch ${TEST_DIR}/main-fake.tf

# set the TEST_COMMAND by defining a valid .tf file in the root directory
TEST_COMMAND="pre-commit try-repo --ref v1.62.3 https://github.com/antonbabenko/pre-commit-terraform terrascan --files ${TEST_DIR}/main-fake.tf"

# run the test
./hooks_performance_test.sh "$TEST_NUM" "$TEST_COMMAND" "$TEST_DIR" "$TEST_DESCRIPTION" "$RAW_TEST_RESULTS_FILE_NAME"

I made a test in a fake TF project (I copied and pasted the infra code a few times as you suggested) with 189 directories, 610 files:

5 runs 'terrascan for big TF repo with pre-commit-terraform v1.62.3'

time command	max	min	mean	median
users seconds	6.67	6.08	6.476	6.56
system seconds	0.4	0.3	0.358	0.37
CPU %	165	159	162.6	163
Total time	4.33	3.89	4.192	4.29

Run details

• Test Start: Thu Dec 23 09:12:35 UTC 2021
• Test End: Thu Dec 23 09:12:56 UTC 2021

Variable name	Value
TEST_NUM	5
TEST_COMMAND	pre-commit try-repo --ref v1.62.3 https://github.com/antonbabenko/pre-commit-terraform terrascan --files /home/carlos/devel/azure/terraform/test/terraform_big_repo/main-fake.tf
TEST_DIR	/home/carlos/devel/azure/terraform/test/terraform_big_repo
TEST_DESCRIPTION	5 runs 'terrascan for big TF repo with pre-commit-terraform v1.62.3'
RAW_TEST_RESULTS_FILE_NAME	terrascan_big_terrfaform_repo

Memory info (head -n 6 /proc/meminfo):

MemTotal:       26034676 kB
MemFree:        20826292 kB
MemAvailable:   23280460 kB
Buffers:          232212 kB
Cached:          2866148 kB
SwapCached:            0 kB

CPU info:

Real procs: 4
Virtual (hyper-threading) procs: 8

processor	: 7
vendor_id	: GenuineIntel
cpu family	: 6
model		: 140
model name	: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
stepping	: 1
microcode	: 0xffffffff
cpu MHz		: 2803.198
cache size	: 12288 KB
physical id	: 0
siblings	: 8
core id		: 3
cpu cores	: 4
apicid		: 7
initial apicid	: 7
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology tsc_reliable nonstop_tsc cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid avx512f avx512dq rdseed adx smap avx512ifma clflushopt clwb avx512cd sha_ni avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves avx512vbmi umip avx512_vbmi2 gfni vaes vpclmulqdq avx512_vnni avx512_bitalg avx512_vpopcntdq rdpid fsrm avx512_vp2intersect flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs
bogomips	: 5606.39
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

[MaxymVlasov]

The problem is not that hooks run recursively (at least, not in this issue), problem is that the same check runs multiply times.

Not sure why, but 1 recursive check works much faster than for_each dir in the repo, no matter, with --non-recursive or without.

5 runs 'with --non-recursive'

time command	max	min	mean	median
users seconds	130.08	116.22	121.908	121.41
system seconds	8.51	7.01	7.532	7.41
CPU %	165	160	162	161
Total time	83.68	77.01	79.642	79.56

Run details

• Test Start: Thu Dec 23 14:35:52 UTC 2021
• Test End: Thu Dec 23 14:42:30 UTC 2021

Variable name	Value
TEST_NUM	5
TEST_COMMAND	pre-commit try-repo -a /mnt/c/Users/vm/code/open-source/pre-commit-terraform terrascan
TEST_DIR	/home/vm/code/Oslo
TEST_DESCRIPTION	5 runs 'with --non-recursive'
RAW_TEST_RESULTS_FILE_NAME	pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         7107868 kB
MemAvailable:    9418472 kB
Buffers:          281768 kB
Cached:          2055276 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.009
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'as is'

time command	max	min	mean	median
users seconds	130.08	105.84	115.695	114.535
system seconds	8.51	4.89	6.566	6.605
CPU %	168	160	163.5	163.5
Total time	83.68	67.76	74.619	74.68

Run details

• Test Start: Thu Dec 23 16:58:08 UTC 2021
• Test End: Thu Dec 23 17:03:56 UTC 2021

Variable name	Value
TEST_NUM	5
TEST_COMMAND	pre-commit try-repo -a /mnt/c/Users/vm/code/open-source/pre-commit-terraform terrascan
TEST_DIR	/home/vm/code/Oslo
TEST_DESCRIPTION	5 runs 'as is'
RAW_TEST_RESULTS_FILE_NAME	pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         8346212 kB
MemAvailable:   10672940 kB
Buffers:          295600 kB
Cached:          2057220 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.009
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

5 runs 'one recurcive check'

time command	max	min	mean	median
users seconds	10.25	9.24	9.682	9.44
system seconds	1.49	1.07	1.316	1.38
CPU %	94	85	91	91
Total time	12.44	11.64	12.016	11.93

Run details

• Test Start: Thu Dec 23 17:15:22 UTC 2021
• Test End: Thu Dec 23 17:16:22 UTC 2021

Variable name	Value
TEST_NUM	5
TEST_COMMAND	pre-commit try-repo -a /mnt/c/Users/vm/code/open-source/pre-commit-terraform terrascan
TEST_DIR	/home/vm/code/Oslo
TEST_DESCRIPTION	5 runs 'one recurcive check'
RAW_TEST_RESULTS_FILE_NAME	pr290

Memory info (head -n 6 /proc/meminfo):

MemTotal:       12765352 kB
MemFree:         8090392 kB
MemAvailable:   10445696 kB
Buffers:          303004 kB
Cached:          2073544 kB
SwapCached:            0 kB

CPU info:

Real procs: 6
Virtual (hyper-threading) procs: 12

processor	: 11
vendor_id	: GenuineIntel
cpu family	: 6
model		: 165
model name	: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
stepping	: 2
microcode	: 0xffffffff
cpu MHz		: 2712.009
cache size	: 12288 KB
physical id	: 0
siblings	: 12
core id		: 5
cpu cores	: 6
apicid		: 11
initial apicid	: 11
fpu		: yes
fpu_exception	: yes
cpuid level	: 21
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology cpuid pni pclmulqdq ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt xsaveopt xsavec xgetbv1 xsaves flush_l1d arch_capabilities
bugs		: spectre_v1 spectre_v2 spec_store_bypass swapgs itlb_multihit
bogomips	: 5424.01
clflush size	: 64
cache_alignment	: 64
address sizes	: 39 bits physical, 48 bits virtual
power management:

The main goal of this issue is to find how to check what args specified to pre-commit, not to hook.

This means that need to check pre-commit.com docs, maybe code, and if needed - open issue to repo.
And then write a check that will check is --all or -a passed and return true of false

[carlosbustillordguez]

Not sure why, but 1 recursive check works much faster than for_each dir in the repo, no matter, with --non-recursive or without.

I noted that inspecting by directories with terrascan scan is much faster than scanning each file (terrascan scan by default inspect directory but also supports inspections by files). You are right with the above pointing.

I think we should avoid the --non-recursive argument, otherwise we can get the following error when no .tf files are present in the root directory:

terrascan scan -i terraform -t azure --non-recursive
2021-12-27T18:56:45.841+0100    error   cli/run.go:132  scan run failed{error 26 0  1 error occurred:
        * directory '/home/carlos/devel/azure/terraform/test/terraform_big_repo' has no terraform config files

The main goal of this issue is to find how to check what args specified to pre-commit, not to hook.
This means that need to check pre-commit.com docs, maybe code, and if needed - open issue to repo.
And then write a check that will check is --all or -a passed and return true of false

Regarding this, I opened an issue in the pre-comit project to know how we can check for --all | -a inside the hook, but:

there isn't a way to do that, somewhat intentionally -- the hook shouldn't care about how it is called, just that it receives a list of filenames

See pre-commit/pre-commit#2172 (comment) for details.

I have implemented a basic solution around that (I need to make some tests to see if all possible cases are covered and handled properly):

https://github.com/carlosbustillordguez/pre-commit-terraform/blob/227f6203e849b97a24daae54d99bee100eae7a36/terrascan.sh#L21-L29

@MaxymVlasov meantime could you test that branch against your repo to see if the performance is improved?

I will back later, with some performance tests and more comments.