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

fake change #12

Closed
wants to merge 8 commits into from
Closed

fake change #12

wants to merge 8 commits into from

Conversation

msridhar
Copy link
Owner

@msridhar msridhar commented Jun 9, 2024

No description provided.

Copy link

Main Branch:

Benchmark                          Mode  Cnt   Score   Error  Units
AutodisposeBenchmark.compile      thrpt   25   9.130 ± 0.122  ops/s
CaffeineBenchmark.compile         thrpt   25   2.117 ± 0.018  ops/s
DFlowMicroBenchmark.compile       thrpt   25  24.554 ± 0.365  ops/s
NullawayReleaseBenchmark.compile  thrpt   25   1.288 ± 0.017  ops/s

With This PR:

Benchmark                          Mode  Cnt   Score   Error  Units
AutodisposeBenchmark.compile      thrpt   25   9.098 ± 0.063  ops/s
CaffeineBenchmark.compile         thrpt   25   2.112 ± 0.026  ops/s
DFlowMicroBenchmark.compile       thrpt   25  24.708 ± 0.211  ops/s
NullawayReleaseBenchmark.compile  thrpt   25   1.244 ± 0.048  ops/s

msridhar added a commit to uber/NullAway that referenced this pull request Jun 12, 2024
Fixes #968 

The key difference is now the benchmarking job only starts when the
label with the name `run-benchmarks` is added to a PR. According to [the
docs](https://docs.github.com/en/issues/using-labels-and-milestones-to-track-work/managing-labels#applying-a-label)
only those with triage access to the repository can add or remove a
label. In contrast, anyone can comment on an issue, which made our
previous technique for kick-starting the benchmarks unsafe. Before
adding the `run-benchmarks` label, a PR should be reviewed to check for
malicious code.

It is impossible to test this workflow without first merging it to the
main branch. However, I did test it using a PR on my fork, and confirmed
it could comment back the benchmark results like before:

msridhar#12

After merging this PR we'll also have to add some credentials to the
main NullAway repo to make this work. But first we should review, land,
and then see that it fails as expected without the credentials.

Compared to [the earlier workflow
file](https://github.com/uber/NullAway/blob/87ec10d4f26630d4bb91aefe5ff7c0fc181f030a/.github/workflows/jmh-benchmark.yml),
beyond changing to use labeling, I updated the versions of some external
GitHub actions we are using.
@msridhar msridhar closed this Jun 29, 2024
@msridhar msridhar deleted the dummy-fork-pr branch June 29, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant