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

Switch release builds to the Roslyn compiler on mono 5.10 #16345

Merged
merged 5 commits into from Mar 30, 2019

Conversation

@pchote
Copy link
Member

commented Mar 23, 2019

This PR updates our Travis build platform to newer versions of Ubuntu, Mono, and NSIS, and switches from the old mcs to the csc, aka Roslyn, complier. This takes us another step towards unifying our toolchain around modern .NET standards and eventually also supporting .NET core.

Depends on #16319.
Another major step towards #15954.
May help with #14182.

I have kept the Makefile targeting C#5, but we can consider (in a future issue/PR!) raising this and adopting some of the newer language features.

The changes in #16316, #16319, #16324 mean that this should not change the runtime requirements for players, but this will impact downstream packaging (ping @diddledan, @Unrud, @fusion809, @svenstaro) and anyone wanting to compile and run from source - particularly on Ubuntu, Debian, or Fedora.

Updating mono and switching to Roslyn should, in principle, produce more efficient builds with better performance. #8153 and #8282 demonstrated that its not that simple, however, and that stumbling into this blindly may cause significant perf regressions.

I'm PRing this as a draft to raise awareness and encourage people to start testing. We must properly benchmark this and resolve any potential performance regressions before merging.

Test builds are available from https://github.com/pchote/OpenRA/releases/tag/pkgtest-20190323.
At some point soon I will see how much of this can be backported to release-20190314 so that we can do before/after benchmarks using RAGL or other game replays.

@pchote

This comment has been minimized.

Copy link
Member Author

commented Mar 24, 2019

An initial test by @teinarss showed the performance regression that I was half expecting to see.

I've hopefully addressed these by adding a new commit that flips the debug flag to false by default. Local testing suggests that this doesn't impact the quality of the stacktraces, and @RoosterDragon's explanation on #8282 suggests that the risk of optimization-induced desyncs is low.

Once we're happy that perf is back at expected levels I will generate a release-20190314-compatible build, and we can test for desyncs by running standard-release replays on the roslyn+optimization-compiled build.

Updated builds have been pushed to the same tag above.

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2019

CPU perfs
image

Rendering
image

@pchote

This comment has been minimized.

Copy link
Member Author

commented Mar 24, 2019

https://github.com/pchote/OpenRA/releases/tag/pkgtest-release-roslyn contains release-20190314 builds compiled using roslyn - this can/should be used to test for desyncs and more robust performance comparisons.

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2019

CPU
image

Render
image

@pchote pchote force-pushed the pchote:build-platform-update branch from 11b22fb to f37e37c Mar 26, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Rebased as the dependencies have now been merged.

@pchote

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

  1. I played through about 30 RAGL masters replays using the backported release-20190314 test build in my Windows 10 VM. No desyncs, as expected.
  2. @teinarss's rough profiling shows no significant performance degradation.

I think this sufficiently covers the regression risk, so i'm now satisfied and opening this up for review and merge.

@pchote pchote marked this pull request as ready for review Mar 29, 2019

@RoosterDragon
Copy link
Member

left a comment

Performance of the test package certainly matches a local Release build via MSBuild/Visual Studio (and a debug build is noticeably slower).

@obrakmann
Copy link
Contributor

left a comment

Compiles, runs, debugging still works with MD, packaging works as well, looks ok to me

INSTALL.md Outdated Show resolved Hide resolved

@pchote pchote force-pushed the pchote:build-platform-update branch from f37e37c to dfb1b5d Mar 30, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2019

Fixed and rebased.

@obrakmann
Copy link
Contributor

left a comment

👍

@obrakmann obrakmann merged commit 9cbf082 into OpenRA:bleed Mar 30, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@obrakmann

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.