-
Notifications
You must be signed in to change notification settings - Fork 1k
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
AOT Compilation Canary #7458
base: v1.6
Are you sure you want to change the base?
AOT Compilation Canary #7458
Conversation
Derivative of what we've done on TurboMqtt: petabridge/TurboMqtt#231 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detailed my changes and how the canary works
inputs: | ||
targetType: 'inline' | ||
script: | | ||
./tools/test-aot-compatibility.ps1 net8.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses a PowerShell script to tabulate the total number of AOT trim warnings and throw an exception if they're above 0 - taken from one of Microsoft's blogs on AOT support for library authors
@@ -152,3 +152,15 @@ jobs: | |||
scriptArgs: CreateNuget nugetprerelease=dev incremental | |||
outputDirectory: "bin/nuget" | |||
artifactName: "nuget_pack-$(Build.BuildId)" | |||
|
|||
- template: azure.aot.template.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do AOT on both Windows and Linux just in-case there are platform-specific issues. Should never happen, but you never know.
{ | ||
public AotReceiveActor() | ||
{ | ||
ReceiveAny(o => Sender.Tell(o)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We know ReceiveActor
s use the ExpressionCompiler
in v1.5 and earlier, which should trip AOT warnings. I might need to add a stricter Receive<string>
with a predicate prior to this one just to make sure that code gets exercised properly in the test,
|
||
namespace Akka.AOT.App.Actors; | ||
|
||
public class AotUntypedActor : UntypedActor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UntypedActor
is already AOT-friendly, but we should still see that show up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need to add more cases to the canary over time, such as:
IStash
supportIWithTimers
support- Custom dispatchers
- Custom mailboxes
- Routers
Just to gradually add full trim coverage to everything in the default Akka
library, minus default serialization (which we know will not work)
<IsPackable>false</IsPackable> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Label="AotSettings"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all the MSBUILD settings needed to get the compiler to produce the AOT warnings we're looking for
{ | ||
static async Task Main(string[] args) | ||
{ | ||
var system = ActorSystem.Create("MySystem"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does some end to end messaging and uses the default configuration loading, which currently triggers a number of AOT warnings due to how we load Type
s from HOCON strings - those are all issues on the #7246 epic we will have to gradually fix before this spec can pass.
Not going to be able to merge this until we make significant progress on #7246 |
We could, in theory, merge this and just remove it from the CI/CD pipeline so we can get incremental validation locally while we're implementing the AOT epic - would that be a good idea? cc @akkadotnet/core |
Changes
AOT Canary for the Core
Akka
module - we'll have to gradually expand our AOT support as we're able to, but everything starts with this.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):