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

Refactor to avoid loading System.ServiceModel assembly #2767

Merged
merged 2 commits into from
May 13, 2019

Conversation

slozier
Copy link
Contributor

@slozier slozier commented May 10, 2019

When WCF is not used the System.ServiceModel assembly is not required. With this refactoring the runtime will avoid loading the assembly if WCF if not used.

Edit: My main motivation behind this change it to be able to use CefSharp with .NET Core 3.0. While in the long term it may be better to generate .NET Core specific assemblies, this change allows .NET Core which does not have System.ServiceModel to load the .NET Framework assemblies. As a side effect it also results in a slightly smaller memory footprint for .NET Framework applications that don't use WCF.

I have tested this change by using the NuGet packages (CefSharp.Common/CefSharp.WinForms generated using build.ps1 vs2017) and checking the loaded assemblies when running a program in Release mode. I've also tested it with a .NET Core 3.0 application and it loads up properly (instead of just crashing).

@AppVeyorBot
Copy link

@amaitland
Copy link
Member

Please update the description to include details on the reason for this change and how you've tested it. Overall looks like a simple solution 👍

Code wise please add comments as to the reason for the changes, make sure the methods are removed later as part of any code cleanup effort, they're only called once so they'd be a candidate to be removed. Did you test with a release build? The c# compiler would typically inline such a change as part of an optimisation effort, quick search and https://stackoverflow.com/questions/3329214/is-it-possible-to-force-a-function-not-to-be-inlined suggests it might be possible to specify noinline on a function in vc++, I haven't looked into this in any detail.

@AppVeyorBot
Copy link

@slozier
Copy link
Contributor Author

slozier commented May 13, 2019

@amaitland Thanks for the feedback. Updated as requested.

@amaitland amaitland added this to the 75.0.0 milestone May 13, 2019
@amaitland
Copy link
Member

Great, thanks 👍

@amaitland amaitland merged commit 203d675 into cefsharp:master May 13, 2019
@slozier slozier deleted the servicemodel branch May 13, 2019 20:08
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

Successfully merging this pull request may close these issues.

3 participants