Skip to content

fake change#12

Closed
msridhar wants to merge 8 commits intomasterfrom
dummy-fork-pr
Closed

fake change#12
msridhar wants to merge 8 commits intomasterfrom
dummy-fork-pr

Conversation

@msridhar
Copy link
Copy Markdown
Owner

@msridhar msridhar commented Jun 9, 2024

No description provided.

@github-actions
Copy link
Copy Markdown

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