project-oak / silveroak

Formal specification and verification of hardware, especially for security and privacy.
Apache License 2.0
123 stars 20 forks source link

Speed up CI with selective runs #887

Closed jadephilipoom closed 3 years ago

jadephilipoom commented 3 years ago

Our CI currently runs 10 jobs: it builds the core cava library, and then in parallel builds 9 other things that depend on that. Currently the checks take about 15 minutes, 4 minutes to build cava and then 11 minutes to build silveroak-opentitan/aes and run the rather extensive AES tests. However, at the moment, the core library isn't changing all that often, since DSL development is happening in investigations/cava2, and we have no need to run that costly 11 minute job for most PRs. Perhaps we could cache the result of building cava from previous runs, like we do for third_party, and if the PR hasn't changed cava we use the cached version and only run the jobs corresponding to directories that had changes.

A more involved option for speeding things up would be to separate the AES proofs from the circuit implementation and make that one job into 3: one fairly quick job to build the implementations, and 2 slow ones to run the tests and build the proofs. But the 2 slow jobs could then run in parallel.

blaxill commented 3 years ago

Caching cava/ has been added in #892

jadephilipoom commented 3 years ago

To clarify -- is it now the case that we only run non-cava jobs when the code in those directories changes or cava changes? If not, we might want to reopen the issue. The main point of caching cava, from my perspective, is to enable us to avoid the slow AES check when we change completely unrelated code.

blaxill commented 3 years ago

The rebuild of third_party/ and cava/ are the only ones that get cached/skipped, but the AES builds are not required before you can merge. Currently the checks that have to complete are

Perhaps we can reopen this issue for caching or only running AES when there are relevant changes, but at this point you shouldn't need to wait for AES before being able to merge.

jadephilipoom commented 3 years ago

Ah, okay, cool. If the AES build is no longer required to merge, then I see no need to reopen the issue. Thanks!