-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
[dotnet] Trim away CDP when publishing AOT apps #15217
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
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
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.
Nice done!
Failures not related to changes //javascript/node/selenium-webdriver:test-bidi-browsingcontext-inspector-test.js-chrome
//javascript/node/selenium-webdriver:test-bidi-browsingcontext-inspector-test.js-firefox
//py:common-chrome-bidi-test/selenium/webdriver/common/bidi_tests.py
//rb/spec/integration/selenium/webdriver:action_builder-chrome-remote
//rb/spec/integration/selenium/webdriver:action_builder-firefox-remote
//rb/spec/integration/selenium/webdriver:driver-chrome
//rb/spec/integration/selenium/webdriver:driver-chrome-remote
//rb/spec/integration/selenium/webdriver:select-chrome-remote |
@RenderMichael Big brain is right about thread-safe: if we apply |
@nvborisenko Unfortunately Lazy is not too AOT-friendly, anything captured in the factory is stuck. It doesn’t matter too much usually (it’s usually not too complex, the Lazy is usually used somewhere anyway, etc). This was a very special case. |
But in future if we really want to make some |
Like here in BiDi namespace: https://github.com/SeleniumHQ/selenium/blob/trunk/dotnet/src/webdriver/BiDi/BiDi.cs. I use |
Hmm, that might actually be a good place to do the same trick. I would write a sample app that uses only a small part of the BiDi commands, and try replacing the Lazy field with a property that creates the value on initialization with |
I asked big brain: To replace Lazy in AOT (Ahead-of-Time) environments like Unity IL2CPP or .NET Native, you need a solution that avoids reflection. Here's an AOT-friendly approach using manual lazy initialization:
Even when using Lazy with a factory delegate, some AOT environments may still restrict its use due to internal synchronization mechanisms.
public class AotFriendlyLazy<T>
{
private readonly Func<T> _factory;
private T _value;
private bool _isInitialized;
private readonly object _lock = new object();
public AotFriendlyLazy(Func<T> factory)
{
_factory = factory ?? throw new ArgumentNullException(nameof(factory));
}
public T Value
{
get
{
if (!_isInitialized)
{
lock (_lock)
{
if (!_isInitialized)
{
_value = _factory();
_isInitialized = true;
}
}
}
return _value;
}
}
} Usage Example: private readonly AotFriendlyLazy<ExpensiveResource> _resource =
new AotFriendlyLazy<ExpensiveResource>(() => new ExpensiveResource());
public ExpensiveResource Resource => _resource.Value;
Thread Safety: Uses double-check locking to ensure thread safety. AOT Compatibility: Avoids reflection and dynamic code generation.
// Works if Lazy<T>'s non-reflection code paths are AOT-friendly
private readonly Lazy<ExpensiveResource> _resource =
new Lazy<ExpensiveResource>(() => new ExpensiveResource()); LazyInitializer: private ExpensiveResource _resource;
public ExpensiveResource Resource =>
LazyInitializer.EnsureInitialized(ref _resource, () => new ExpensiveResource());
.NET Native/UWP: Avoids reflection restrictions. High-Performance Scenarios: Manual control over initialization logic. This pattern ensures AOT compatibility while retaining the "initialize-on-first-use" behavior of Lazy. |
What I know: |
The big brain is not wrong there, about AOT compatibility. I think the normal Lazy is AOT-safe, this PR solves another issue: AOT rooting. Anything in the Lazy’s factory is rooted by the AOT compiler as “something which may be invoked”. This means AOT trimming cannot eliminate the NetworkManager constructor, and that constructor also roots the entire DevTools functionality. That’s megabytes of code that is never called. Lazy cannot do this. If we had a |
@nvborisenko I tried it out. Just like I thought, it didn't save anything. ( This PR was a special case where the constructor, by itself, was very heavy. |
Description
Removes unnecessary AOT warnings from the user's eyes, and trims away a lot of code unless it is directly used.
Motivation and Context
Size and warning savings
Sample app:
Size before PR: 8.9 MB

Size after PR: 6.5 MB

Size savings: 2.3 MB

Types of changes
Checklist
PR Type
Enhancement
Description
Optimized the
WebDriver
class to reduce application size.Deferred initialization of
NetworkManager
to save resources.Removed unused or redundant code for cleaner implementation.
Changes walkthrough 📝
WebDriver.cs
Optimize and streamline `WebDriver` initialization
dotnet/src/webdriver/WebDriver.cs
NetworkManager
.NetworkManager
initialization using a null-coalescingassignment.
Network
property setter.