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

Investigate Appveyor alternatives for more parallel builds: Github Actions; Travis; CircleCI #4131

Closed
derekbruening opened this issue Feb 24, 2020 · 7 comments

Comments

@derekbruening
Copy link
Contributor

Appveyor's big downside is the single sequential job serialized across both dynamorio and drmemory repositories and across PR's and master merges. One single job is now down to ~23-24 mins thanks to #4059 which is not that different from a Travis invocation, but the lack of parallelism does cause pain.

Possible alternatives that now include Windows (all quite recently):

  • Github Actions
  • Travis: looks like it has only VS2017 and very few packages and is in early stages
  • CircleCI
@derekbruening
Copy link
Contributor Author

Xref #4549 where we almost certainly need to migrate from Travis rather urgently: and Appveyor is a possibility, in a reversal of this issue.

@derekbruening derekbruening self-assigned this Dec 1, 2020
@derekbruening
Copy link
Contributor Author

Assigning to me since I started some work on Dr. Memory's Windows CI on Github Actions which overlaps with the work for DR: DynamoRIO/drmemory#2326

@derekbruening
Copy link
Contributor Author

If we successfully add auto-cancellation (#4585) to Github Actions that could be another big reason to move from Appveyor, avoiding the current pain: https://github.com/DynamoRIO/dynamorio/wiki/Test-Suite#ci-resources-and-appveyor-global-serialization

derekbruening added a commit that referenced this issue Dec 5, 2020
Adds two separate jobs on Windows, 32-bit and 64-bit.
We target a VS2017 Win10-1607 image to match what we had on Appveyor.

Modifies the runsuite_wrapper.pl script to *not* fork, since there is
no Cygwin perl available by default.  Instead, we tee to a file.

This is based on the similar work in progress for Dr. Memory:
DynamoRIO/drmemory#23

Issue: #4131
derekbruening added a commit that referenced this issue Dec 14, 2020
Splits off about half of
suite/tests/api/ir_x86_4args_avx512_evex_mask_B.h into a new _C file
to avoid an out-of-memory in VS2017 on Github Actions.
The recent AVX-512 broadcast additions for #4534 in PR #4577 seem to
have pushed it over the edge.

Issue: #4534, #4610, #4131, #4549
Fixes #4610
derekbruening added a commit that referenced this issue Dec 14, 2020
Splits off about half of
suite/tests/api/ir_x86_4args_avx512_evex_mask_B.h into a new _C file
to avoid an out-of-memory in VS2017 on Github Actions.
The recent AVX-512 broadcast additions for #4534 in PR #4577 seem to
have pushed it over the edge.

Issue: #4534, #4610, #4131, #4549
Fixes #4610
derekbruening added a commit that referenced this issue Dec 15, 2020
Splits off about half of
suite/tests/api/ir_x86_4args_avx512_evex_mask_B.h into a new _C file
to avoid an out-of-memory in VS2017 on Github Actions.
The recent AVX-512 broadcast additions for #4534 in PR #4577 seem to
have pushed it over the edge.

Issue: #4534, #4610, #4131, #4549
Fixes #4610
derekbruening added a commit that referenced this issue Dec 15, 2020
Adds three separate jobs on Windows: 32-bit debug with tests, 64-bit debug with tests,
and a job that creates 3 builds without tests: release-32, release-64, and vps-32-debug.
Adds corresponding flags to the script layers to accomplish this build splitting.

We target a VS2017 Win10-1607 image to match what we had on Appveyor.

Modifies the runsuite_wrapper.pl script to *not* fork, since there is
no Cygwin perl available by default.  Instead, we tee to a file.
Updates checks for Cygwin to apply to native Windows perl as well.

Includes attempts to improve two tests:
+ Add error code printing for drx-test
+ Generalize api.symtest golden output to handle extra operators found

Other failing tests were either already on the ignore list or we add them here.
The long list of tests ignored for #4058 is culled to remove tests never observed
to fail on GA.

Removes APPVEYOR_PULL_REQUEST_NUMBER-driven build skips now that we have
a separate build job.  Sets a CI_TRIGGER env var and use it to shrink 32-bit tests in the
same manner as before for Appveyor PR runs.

This is based on the similar work for Dr. Memory:
DynamoRIO/drmemory#2326

Issue: #4131
@derekbruening
Copy link
Contributor Author

I removed the Appveyor webhook and swapped to the GA jobs as the ones that are required to merge.

derekbruening added a commit that referenced this issue Dec 15, 2020
Now that we have switched our Windows CI to Github Actions we no
longer need an Appveyor config file.

Issue: #4131
@derekbruening
Copy link
Contributor Author

We now have every single CI job under ~15 mins (64-bit Windows is sometimes 16 mins or so). It's pretty nice. Add in no serialization across PR's or repos, and auto-cancellation of prior jobs, and it's really nice.

Unfortunately there are still a number of tests failing on Windows which are just on an ignore list, but that's how it was with Appveyor when we moved to Win10 VS2017. Hopefully someone will get some time to fix a few of them. I think it would be best to file separate issues instead of lumping them all under here or #4058 so I will take that as an action item to file them all separately (the set failing on GA is slightly different from Appveyor).

@derekbruening
Copy link
Contributor Author

Separate issues for test failures include #2246, #4616, #4617, #4618, #4621, #4622.
That covers the highest priority ones. Closing this now. Leaving #4058 open until all of them are separated out or fixed.

@derekbruening
Copy link
Contributor Author

#4619 also.

derekbruening added a commit that referenced this issue Dec 15, 2020
Updates the Windows CI ignore list bug numbers to reflect the
newly-filed individual issues.

Re-adds client.drwrap-test-detach to the ignore list for 32-bit.

Issue: #2246, #4616, #4617, #4618, #4619, #4621, #4622, #4131, #4058
derekbruening added a commit that referenced this issue Dec 15, 2020
Updates the Windows CI ignore list bug numbers to reflect the
newly-filed individual issues.

Re-adds client.drwrap-test-detach to the ignore list for 32-bit,
and adds histogram-offline to 64-bit (both were already there for
the other bitwidth).

Issue: #2246, #4616, #4617, #4618, #4619, #4621, #4622, #4131, #4058
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant