Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR Testing: Build Performance (CI Refactor) #16

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

sgammon
Copy link
Owner

@sgammon sgammon commented Mar 10, 2024

Filed to run google#7093 against the changes in google#7089.

step-security-bot and others added 15 commits March 7, 2024 21:54
- chore: apply 'Harden Runner' auditing to all ci tasks
- chore: apply `persist-credentials: false` to checkout tasks
- chore: publish dependency graph and add dependency review check
- chore: add codeql scan job (temp)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.1.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.6.0...b4ffde6)

Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 2.5.1 to 4.1.3.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](actions/dependency-review-action@0efb1d1...9129d7d)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: actions/dependency-review-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: StepSecurity Bot <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This change refactors the main CI workflow into two new workflows, `on.pr.yml` and
`on.push.yml`, which each call into the exiting CI job as a reusable workflow.

This has the nice benefit of putting all tests, checks, builds, etc., on one screen
during development on GitHub, allows customization of the PR vs. push flow, and yet
keeps behavior fully consistent between the two.

- chore: move ci jobs to `workflow_call` trigger
- chore: add entrypoint jobs for PR and Push events
- chore: cleanup permissions and dispatch checks/tests

Signed-off-by: Sam Gammon <[email protected]>
This changeset switches the StepSecurity hardening action to enforced mode, where
previously it was running in `audit` mode. Now, audit logs have been gathered and
it is time to seal off the list of accessible network endpoints for a given job.

- chore: gather and apply network endpoints for each job
- chore: move to `block` mode for `egress-policy` in `step-security/harden-runner`

Signed-off-by: Sam Gammon <[email protected]>
This changeset adds SLSA 3+ provenance support to the workflow. The main CI run has now been
split into two: `ci.build.yml`, which only builds the library and is provenance-capable, and
`ci.test.yml`, which is the previous CI logic.

The regular build logic is applied only on push, and can be applied on PRs too, with publish
of provenance material turned off. The test suite is invoked from PRs.

The workflows have been split into build/test phases to avoid publishing provenance data and
GitHub artifacts for build matrix outputs. JARs are uniform across OS targets, so there is no
need to gather and publish for more than Ubuntu.

- feat: add slsa support to build workflow
- chore: split `test` into `build` and `test` workflows
- chore: use new workflows (build/test) from push/pr triggers

Signed-off-by: Sam Gammon <[email protected]>
Adds two build parameters
- `publishing.repository.snapshots`: Snapshot repo to deploy to
- `publishing.repository.releases`: Releases repo to deploy to

Both default to their current values, Sonatype. This small inert
change allows a fork to easily publish to a different repository
without resorting to a code change.

Signed-off-by: Sam Gammon <[email protected]>
This changeset adds the Maven Sigstore plugin for use during
publishing to Sonatype and other public repositories.

- chore: add sigstore plugin to build

Signed-off-by: Sam Gammon <[email protected]>
Fails the build if any downloaded dependencies fail their checksum
verification.

- chore: add `--strict-checksums` flag to `mvnw` calls in ci
Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
…precondition checks.

It's not that we're not going to make such calls illegal, I promise :) I mean, we certainly aren't going to _in general_, but I am tempted for `com.google.common`, as discussed on cl/372346107 :) (It would have caught the problem of cl/612591080!) I'm testing what would happen if we did do it for `com.google.common` in case it shakes out any more bugs.

It does reveal that I didn't complete the cleanup of cl/612591080. And it reveals a few places where we'd normally use `requireNonNull`, since the checks aren't "preconditions" in the sense of "the caller did something wrong" (from cl/15376243 and cl/526930990). I've made those changes. (I would have made some more changes if I had tried to address more of `com.google.common`. But I stuck to the "main" packages, and I didn't even fix enough errors to see full results.)

Honestly, the more interesting thing that this exercise revealed was that there are more cases in which I'm especially sympathetic to calling `checkNotNull` on nullable values:
- `DummyProxy` is making an `InvocationHandler` perform automatic precondition tests based on annotations on the interface it's implementing.
- `EqualsTester` and Truth have permissive signatures because they're test utilities, as documented in cl/578260904 and discussed during the Truth CLs.

And the yet more interesting thing that it revealed is that we may want to use `@NonNull` here in the future, similar to what we've discussed in google#6824.

RELNOTES=n/a
PiperOrigin-RevId: 614074533
This is another baby step toward google#2970.

RELNOTES=n/a
PiperOrigin-RevId: 614287410
- chore: drop jdk srczip dependencies
- chore: cleanup comments and config related to srczip

Signed-off-by: Sam Gammon <[email protected]>
This changeset optimizes the Guava build significantly by enabling
parallel build and test features supported by Maven. With these
flags enabled, only a few tests exhibit flaky behavior; applying a
sensible count of test retries (3) solves the problem.

As a result, the testsuite can now be executed often, because it
takes about 2 minutes to run. Building is also much faster. After
benchmarking different configurations, 2-threads-per-core and
2-test-forks-per-core seems optimal:

```
[INFO] Guava Maven Parent ..................... SUCCESS [  0.121 s]
[INFO] Guava: Google Core Libraries for Java .. SUCCESS [  9.681 s]
[INFO] Guava BOM .............................. SUCCESS [  0.120 s]
[INFO] Guava Testing Library .................. SUCCESS [ 47.883 s]
[INFO] Guava Unit Tests ....................... SUCCESS [01:57 min]  <--
[INFO] Guava GWT compatible libs .............. SUCCESS [  6.909 s]
```

When built and executed serially:
```
[INFO] Guava Maven Parent ..................... SUCCESS [  0.129 s]
[INFO] Guava: Google Core Libraries for Java .. SUCCESS [ 15.653 s]
[INFO] Guava BOM .............................. SUCCESS [  0.064 s]
[INFO] Guava Testing Library .................. SUCCESS [01:26 min]
[INFO] Guava Unit Tests ....................... SUCCESS [06:26 min] <--
[INFO] Guava GWT compatible libs .............. SUCCESS [ 11.092 s]
```

Benchmark hardware:
- Apple M2 Max, 96GB RAM
- macOS Sonoma 14.3.1
- GraalVM CE JVM 21.0.2

```
openjdk version "21.0.2" 2024-01-16
OpenJDK Runtime Environment GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30)
OpenJDK 64-Bit Server VM GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30, mixed mode, sharing)
```

- chore: enable parallel build
- chore: enable parallel test execution
- chore: enable parallel gc for maven
- chore: tune tiered compilation for maven
- chore: tune thread count for maven
- fix: enable test retries (max = 3) for parallel-flaky tests

Signed-off-by: Sam Gammon <[email protected]>
@sgammon sgammon self-assigned this Mar 10, 2024
@sgammon sgammon added the enhancement New feature or request label Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants