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

Allow to filter/opt-in assemblies by name via MSBuild property #30

Open
neon-sunset opened this issue Nov 26, 2023 · 4 comments
Open

Allow to filter/opt-in assemblies by name via MSBuild property #30

neon-sunset opened this issue Nov 26, 2023 · 4 comments

Comments

@neon-sunset
Copy link

neon-sunset commented Nov 26, 2023

It appears that the current way the build task works is somewhat "all or nothing" - either annotate individual methods with [Optimize] or press the big red button and add <DistilAllMethods>.

Turns out the latter completely breaks ILLink on Linux in an otherwise seemingly working application (with what is likely a runaway inlining scenario causing SO).

It would be nice to have a more granular way to choose what to optimize with opt-in and/or opt-out properties in csproj. Alternatively, [Optimize] could be made applicable to assembly instead.

@dubiousconst282
Copy link
Owner

The optimizer will actually only transform methods in the main project assembly where the build task was imported, dependencies and cross-assembly opts are yet to happen. The [Optimize] attribute works per-class too btw, but a more granular project property does sounds nice.

There's currently the --filter CLI switch which allows filtering per method/class, it should be accessible via the DistilExtraArgs property, but it was intended for debugging purposes and it doesn't scale well. For reference, it'd look something like this:

<DistilExtraArgs>--filter !ClassName::MethodName|MethodName2|get_*</DistilExtraArgs>

Also, I think the SO (stack overflow?) crashes might be due to methods with stackallocs being inlined, there are no checks for these yet. (There are some weird gotchas with stackallocs that might lead to other codegen issues. I think it'd be best to completely skip parsing methods with them as a quick workaround, since there isn't much DistIL can do for them anyway atm.)

@neon-sunset
Copy link
Author

Also, I think the SO (stack overflow?) crashes might be due to methods with stackallocs being inlined, there are no checks for these yet. (There are some weird gotchas with stackallocs that might lead to other codegen issues. I think it'd be best to completely skip parsing methods with them as a quick workaround, since there isn't much DistIL can do for them anyway atm.)

Interestingly enough, the crash happens not in the application code itself but in the linker on project publish, which is why I assumed DistIL touches its code. Not sure how to go about troubleshooting it aside from examining IL. The next culprit is optimizations applied to source-generated System.Text.Json code.

@dubiousconst282
Copy link
Owner

I'd be interested in taking a look if you have a repro available.

I suppose the fuzzy bisect script would be very helpful in narrowing down to the methods leading to this issue, I guess it wouldn't be too hard to hack it around the csproj.

@neon-sunset
Copy link
Author

neon-sunset commented Dec 2, 2023

Let me try to repro with a new update first :)

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

No branches or pull requests

2 participants