Skip to content

Adds GitHub workflow to detect the performance regression of ion-java automatically.#373

Merged
linlin-s merged 5 commits intoamazon-ion:masterfrom
linlin-s:master
Aug 10, 2021
Merged

Adds GitHub workflow to detect the performance regression of ion-java automatically.#373
linlin-s merged 5 commits intoamazon-ion:masterfrom
linlin-s:master

Conversation

@linlin-s
Copy link
Contributor

Description:

This work flow will be invoked when there is new ion-java commit gets pushed to the master branch of amzn/ion-java.

Step 1:
Build ion-java from the new commit on GitHub runner.
Step 2:
Build ion-java-benchmark-cli on the same GitHub runner. In this way the benchmark results of ion-java from the new commit will be generated after the benchmark step.
Step 3:
Generate three shapes of test ion data (Ion List | Ion Struct | Nested Ion Struct) by using the sub-command of ion-java-benchmark-cli.
Step 4:
Upload all generated test ion data to artifacts.
Step 5:
For each shape of test data, six ion-java-benchmark-cli invokes will be implemented to finish the benchmark process. After this process, the benchmark results generated from different ion-java-benchmark invoke will be saved in different directories.
Step 6:
Upload the benchmark results to artifacts.
Step 7:
Clean the directory .m2/ which can make sure the second build will be implemented in a clean environment.
The sequence of maven project searching for maven dependencies is searching locally first then searching in maven central. If we build ion-java locally, then the ion-java-benchmark-cli will find the local ion-java first then start the benchmark process. When we try to benchmark different version of ion-java, it is necessary to clean up the .m2/ directory which contains all maven dependencies to make sure ion-java-benchmark-cli will not take the ion-java from the previous build process.
Step 8:
Check out the ion-java from the previous commit by using ‘git checkout HEAD^’ and build ion-java from the previous commit.
Step 9:
Build ion-java-benchmark-cli for the second time to make sure the following benchmark process will be executed to the ion-java from the previous commit.
Step 10:
Benchmark ion-java from the previous commit with the same test ion data and same ion-java-benchmark invokes.
Step 11:
Upload all benchmark results to artifacts.
Step 12:
Iterate all directories in the benchmark results then using API ‘ion-java-benchmark compare’ to compare the benchmark results from different ion-java commits with the same test data and ion-java-benchmark-cli invoke, then generating the comparison report and saving this report under the same directory as the benchmark results.
Step 13:
Detect the performance regression by comparing the scores in comparison reports with thresholds value(The calculating threshold process will be finished when comparing the benchmark results.)
Step 14:
If there is one performance regression detected, the workflow will be failed and users can keep doing research on benchmark results by downloading the artifacts of this workflow.

Performance regression detected test:

Action link:
https://github.com/linlin-s/ion-java/actions/runs/1113837386
Using #368 as an new ion-java commit to invoke the workflow which has heap usage performance regression compares to #369
The results is shown as this:
image

Performance regression not detected test:

Action link:
https://github.com/linlin-s/ion-java/actions/runs/1113787088
Using the ion-java commit without any change to invoke the workflow
The results is shown as this:
image

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@linlin-s linlin-s requested a review from cheqianh August 10, 2021 00:49
@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #373 (5a81449) into master (edf7809) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #373   +/-   ##
=========================================
  Coverage     64.08%   64.08%           
- Complexity     4843     4844    +1     
=========================================
  Files           138      138           
  Lines         21142    21142           
  Branches       3822     3822           
=========================================
  Hits          13548    13548           
  Misses         6256     6256           
  Partials       1338     1338           
Impacted Files Coverage Δ
src/com/amazon/ion/impl/BlockedBuffer.java 50.72% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edf7809...5a81449. Read the comment docs.

cheqianh
cheqianh previously approved these changes Aug 10, 2021
Copy link
Contributor

@cheqianh cheqianh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed in linlin-s#2 (review) and linlin-s#3 🛳️.

One minor comment below, feel free to "squash and merge".


on:
push:
branches: [ master ]
Copy link
Contributor

@cheqianh cheqianh Aug 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

push: branches: [master] seems redundant here since amzn/master always require a pull request before pushing changes into it (by its branch protection rule).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

push: branches: [master] seems redundant here since amzn/master always require a pull request before pushing changes into it (by its branch protection rule).

Good catch, push: branches: [master] will be deleted in the next commit.

tgregg
tgregg previously approved these changes Aug 10, 2021
@linlin-s linlin-s dismissed stale reviews from tgregg and cheqianh via c3af998 August 10, 2021 20:16
@linlin-s linlin-s requested a review from cheqianh August 10, 2021 20:18
cheqianh
cheqianh previously approved these changes Aug 10, 2021
@linlin-s linlin-s merged commit ddaa965 into amazon-ion:master Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants