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

SerializationGuard bloats the binary size on NAOT #87470

Closed
aromaa opened this issue Jun 13, 2023 · 7 comments · Fixed by #89706
Closed

SerializationGuard bloats the binary size on NAOT #87470

aromaa opened this issue Jun 13, 2023 · 7 comments · Fixed by #89706
Labels
area-System.Runtime linkable-framework Issues associated with delivering a linker friendly framework

Comments

@aromaa
Copy link
Contributor

aromaa commented Jun 13, 2023

Currently when calling Process.Start it brings the whole reflection stack with it even when the Switch.System.Runtime.Serialization.SerializationGuard.AllowProcessCreation is specified. This adds about 1MB to the final binary size.

The SerializationGuard seems to use reflection internally to retrieve the switch here which ends up rooting the reflection stack. It would be nice if the whole reflection guard concept would be possible to trim away on NAOT.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 13, 2023
@aromaa
Copy link
Contributor Author

aromaa commented Jun 13, 2023

Related #44369. Noticed it after I changed the title to be more general last minute. I'm more concerned about the Process.Start myself so this might be more actionable as a subset?

@danmoseley
Copy link
Member

Cc @GrabYourPitchforks

@danmoseley danmoseley added the linkable-framework Issues associated with delivering a linker friendly framework label Jun 13, 2023
@ghost
Copy link

ghost commented Jun 13, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Currently when calling Process.Start it brings the whole reflection stack with it even when the Switch.System.Runtime.Serialization.SerializationGuard.AllowProcessCreation is specified. This adds about 1MB to the final binary size.

The SerializationGuard seems to use reflection internally to retrieve the switch here which ends up rooting the reflection stack. It would be nice if the whole reflection guard concept would be possible to trim away on NAOT.

Author: aromaa
Assignees: -
Labels:

area-Serialization, untriaged, linkable-framework

Milestone: -

@ghost
Copy link

ghost commented Jun 13, 2023

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Currently when calling Process.Start it brings the whole reflection stack with it even when the Switch.System.Runtime.Serialization.SerializationGuard.AllowProcessCreation is specified. This adds about 1MB to the final binary size.

The SerializationGuard seems to use reflection internally to retrieve the switch here which ends up rooting the reflection stack. It would be nice if the whole reflection guard concept would be possible to trim away on NAOT.

Author: aromaa
Assignees: -
Labels:

area-Serialization, area-System.Runtime, untriaged, linkable-framework

Milestone: -

@jkotas
Copy link
Member

jkotas commented Jun 13, 2023

It would be nice if the whole reflection guard concept would be possible to trim away on NAOT.

Can we at least introduce a feature switch to allow people who do not want to pay for the serialization guard overheads to opt-out of it?

@stephentoub
Copy link
Member

It would be nice if the whole reflection guard concept would be possible to trim away on NAOT.

Can we at least introduce a feature switch to allow people who do not want to pay for the serialization guard overheads to opt-out of it?

I'd be in favor of deleting it entirely, especially with BinaryFormatter disabled by default in most workloads and scheduled for extinction. Short of that, a feature switch in the meantime makes sense. Could we not just use the existing one for BinaryFormatter to also guard the guard?

@MichalStrehovsky
Copy link
Member

Per #44369 (comment) this is also used in deserialization of DataTable so it's not BinaryFormatter only.

Looks like users can configure DataSet/DataTable deserialization to be a binaryformatter-like security nightmare. I wonder whether that mode should also be obsoleted together with BinaryFormatter.

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jul 31, 2023
Use `UnsafeAccessor` instead of a delegate.

Shrinks the size of a `PublishAot` app that just `Process.Start`s a new process from 2.2 MB to 1.7 MB. It's still half a MB bigger than what I would expect but now it's `Process`'s fault (the `ToString()` brings some serious amount of garbage and we can't practically trim `ToString`).

Fixes dotnet#87470.
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 31, 2023
MichalStrehovsky added a commit that referenced this issue Aug 2, 2023
Use `UnsafeAccessor` instead of a delegate.

Shrinks the size of a `PublishAot` app that just `Process.Start`s a new process from 2.2 MB to 1.7 MB. It's still half a MB bigger than what I would expect but now it's `Process`'s fault (the `ToString()` brings some serious amount of garbage and we can't practically trim `ToString`).

Fixes #87470.
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Aug 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants