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

[API Proposal]: Host.CreateEmptyApplicationBuilder #81280

Closed
eerhardt opened this issue Jan 27, 2023 · 11 comments · Fixed by #81728
Closed

[API Proposal]: Host.CreateEmptyApplicationBuilder #81280

eerhardt opened this issue Jan 27, 2023 · 11 comments · Fixed by #81728
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Hosting

Comments

@eerhardt
Copy link
Member

eerhardt commented Jan 27, 2023

Background and motivation

In .NET 8, we are enabling ASP.NET Minimal APIs to support NativeAOT. As part of this effort, our goal is to get the template app's publish size to 10 MB. To meet this goal, we need to cut unnecessary dependencies in the application.

Some of the unnecessary dependencies come from the Microsoft.Extensions.Hosting.HostApplicationBuilder API. Specifically, the Microsoft.Extensions.Configuration.Json usage for appsettings.json and optional Logging services (Console, Debug, EventSource, EventLog).

We should create a way to create an "empty" HostApplicationBuilder that doesn't bring any of these optional/unnecessary dependencies into the app's closure. Functionally, this would behave the same as new HostApplicationBuilder(new HostApplicationBuilderSettings() { DisableDefaults = true }).

This should cut about 1.3 MB of a NativeAOT published Windows x64 app. Prototyping this change on my machine results the Benchmark app:

Before
15.2 MB (16,007,168 bytes)

After
13.9 MB (14,593,024 bytes)

And 1.4MB on Linux x64:

Before
18.6 MB (19,601,424 bytes)

After
17.2 MB (18,132,768 bytes)

The bulk of this size savings (~1MB) is due to the Console Logger getting trimmed from the app. A follow up issue will be logged to make Console Logging smaller when used/needed.

API Proposal

namespace Microsoft.Extensions.Hosting;

public static class Host
{
        public static HostApplicationBuilder CreateApplicationBuilder();
        public static HostApplicationBuilder CreateApplicationBuilder(string[]? args);
+       public static HostApplicationBuilder CreateEmptyApplicationBuilder(HostApplicationBuilderSettings? settings);
        public static IHostBuilder CreateDefaultBuilder();
        public static IHostBuilder CreateDefaultBuilder(string[]? args);
}
public sealed partial class HostApplicationBuilder
{
    public HostApplicationBuilder() { }
    public HostApplicationBuilder(HostApplicationBuilderSettings? settings) { }
    public HostApplicationBuilder(string[]? args) { }
}

API Usage

var builder = Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings
{
    Args = args,
    ApplicationName = "My App",
    EnvironmentName = "Production",
    ContentRootPath = Environment.CurrentDirectory
    Configuration = new ConfigurationManager(),
});

// do the same things with `builder` as normal
builder.Logging.AddConsole();
builder.Services.AddScoped(_ => new SqliteConnection(connectionString));

IHost host = builder.Build();
host.Run();

Alternative Designs

Create a whole new, duplicate HostApplicationBuilder class that doesn't have these dependencies.

Risks

Users expecting the default behavior of new HostApplicationBuilder() being the same as Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings()).

@eerhardt eerhardt added api-suggestion Early API idea and discussion, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work area-Extensions-Hosting labels Jan 27, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 27, 2023
@ghost
Copy link

ghost commented Jan 27, 2023

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

Issue Details

Background and motivation

In .NET 8, we are enabling ASP.NET Minimal APIs to support NativeAOT. As part of this effort, our goal is to get the template app's publish size to 10 MB. To meet this goal, we need to cut unnecessary dependencies in the application.

Some of the unnecessary dependencies come from the Microsoft.Extensions.Hosting.HostApplicationBuilder API. Specifically, the Microsoft.Extensions.Configuration.Json usage for appsettings.json and optional Logging services (Console, Debug, EventSource, EventLog).

We should create a way to create an "empty" HostApplicationBuilder that doesn't bring any of these optional/unnecessary dependencies into the app's closure. Functionally, this would behave the same as new HostApplicationBuilder(new HostApplicationBuilderSettings() { DisableDefaults = true }).

In prototyping this API, it saves roughly 300 KB in the published Linux x64 app.

API Proposal

namespace Microsoft.Extensions.Hosting;

public static class Host
{
        public static HostApplicationBuilder CreateApplicationBuilder();
        public static HostApplicationBuilder CreateApplicationBuilder(string[]? args);
+       public static HostApplicationBuilder CreateEmptyApplicationBuilder(HostApplicationBuilderSettings? settings);
        public static IHostBuilder CreateDefaultBuilder();
        public static IHostBuilder CreateDefaultBuilder(string[]? args);
}
public sealed partial class HostApplicationBuilder
{
    public HostApplicationBuilder() { }
    public HostApplicationBuilder(HostApplicationBuilderSettings? settings) { }
    public HostApplicationBuilder(string[]? args) { }
}

API Usage

var builder = Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings
{
    Args = args,
    ApplicationName = "My App",
    EnvironmentName = "Production",
    ContentRootPath = Environment.CurrentDirectory
    Configuration = new ConfigurationManager(),
});

// do the same things with `builder` as normal
builder.Logging.AddConsole();
builder.Services.AddScoped(_ => new SqliteConnection(connectionString));

IHost host = builder.Build();
host.Run();

Alternative Designs

Create a whole new, duplicate HostApplicationBuilder class that doesn't have these dependencies.

Risks

Users expecting the default behavior of new HostApplicationBuilder() being the same as Host.CreateEmptyApplicationBuilder(new HostApplicationBuilderSettings()).

Author: eerhardt
Assignees: -
Labels:

api-suggestion, blocking, area-Extensions-Hosting

Milestone: -

@eerhardt
Copy link
Member Author

cc @davidfowl @halter73

@eerhardt eerhardt self-assigned this Jan 27, 2023
@eerhardt eerhardt added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jan 27, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 27, 2023
@terrajobst
Copy link
Member

terrajobst commented Jan 30, 2023

Is Empty the right name when it comes with, for example, DI? Would a better name be CreateMinimalAppBuilder()?

@eerhardt
Copy link
Member Author

"Hosting" implies that there is DI. It is intrinsic to what the Hosting layer provides. A HostApplicationBuilder builds an IHost. IHost has 3 things:

public interface IHost : IDisposable
{
/// <summary>
/// Gets the services configured for the program (for example, using <see cref="M:HostBuilder.ConfigureServices(Action&lt;HostBuilderContext,IServiceCollection&gt;)" />).
/// </summary>
IServiceProvider Services { get; }
/// <summary>
/// Starts the <see cref="IHostedService" /> objects configured for the program.
/// The application will run until interrupted or until <see cref="M:IHostApplicationLifetime.StopApplication()" /> is called.
/// </summary>
/// <param name="cancellationToken">Used to abort program start.</param>
/// <returns>A <see cref="Task"/> that will be completed when the <see cref="IHost"/> starts.</returns>
Task StartAsync(CancellationToken cancellationToken = default);
/// <summary>
/// Attempts to gracefully stop the program.
/// </summary>
/// <param name="cancellationToken">Used to indicate when stop should no longer be graceful.</param>
/// <returns>A <see cref="Task"/> that will be completed when the <see cref="IHost"/> stops.</returns>
Task StopAsync(CancellationToken cancellationToken = default);
}

  1. DI Services
  2. You can Start it
  3. You can Stop it

"Empty" means there are no registered services. i.e. there are no Logging or Configuration providers (or other "optional" services).

@terrajobst
Copy link
Member

"Hosting" implies that there is DI. It is intrinsic to what the Hosting layer provides. [...]
"Empty" means there are no registered services. i.e. there are no Logging or Configuration providers (or other "optional" services).

Gotcha, that makes total sense.

@bartonjs
Copy link
Member

bartonjs commented Jan 31, 2023

Video

Through a lot of adding and removing things during the discussion, we added an overload of CreateApplicationBuilder that takes the settings object for symmetry and a clear path from the existing pattern to the new one. This new overload is a source-breaking change for the null literal, but we believe the number of callers will be small enough that it's not a concern.

namespace Microsoft.Extensions.Hosting;

public static class Host
{
        public static HostApplicationBuilder CreateApplicationBuilder();
+       public static HostApplicationBuilder CreateApplicationBuilder(HostApplicationBuilderSettings? settings);
        public static HostApplicationBuilder CreateApplicationBuilder(string[]? args);
+       public static HostApplicationBuilder CreateEmptyApplicationBuilder(HostApplicationBuilderSettings? settings);
        public static IHostBuilder CreateDefaultBuilder();
        public static IHostBuilder CreateDefaultBuilder(string[]? args);
}
public sealed partial class HostApplicationBuilder
{
    public HostApplicationBuilder() { }
    public HostApplicationBuilder(HostApplicationBuilderSettings? settings) { }
    public HostApplicationBuilder(string[]? args) { }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 31, 2023
@davidfowl
Copy link
Member

I didn't realize the HostApplicationBuilder has a public constructor, how is that different from empty?

@stephentoub
Copy link
Member

I didn't realize the HostApplicationBuilder has a public constructor, how is that different from empty?

HostApplicationBuilder's ctor is identical to CreateApplicationBuilder(), which just calls the ctor, and that constructor does much more than nothing:

/// <summary>
/// Initializes a new instance of the <see cref="HostApplicationBuilder"/> class with pre-configured defaults.
/// </summary>
/// <remarks>
/// The following defaults are applied to the returned <see cref="HostApplicationBuilder"/>:
/// <list type="bullet">
/// <item><description>set the <see cref="IHostEnvironment.ContentRootPath"/> to the result of <see cref="Directory.GetCurrentDirectory()"/></description></item>
/// <item><description>load host <see cref="IConfiguration"/> from "DOTNET_" prefixed environment variables</description></item>
/// <item><description>load host <see cref="IConfiguration"/> from supplied command line args</description></item>
/// <item><description>load app <see cref="IConfiguration"/> from 'appsettings.json' and 'appsettings.[<see cref="IHostEnvironment.EnvironmentName"/>].json'</description></item>
/// <item><description>load app <see cref="IConfiguration"/> from User Secrets when <see cref="IHostEnvironment.EnvironmentName"/> is 'Development' using the entry assembly</description></item>
/// <item><description>load app <see cref="IConfiguration"/> from environment variables</description></item>
/// <item><description>load app <see cref="IConfiguration"/> from supplied command line args</description></item>
/// <item><description>configure the <see cref="ILoggerFactory"/> to log to the console, debug, and event source output</description></item>
/// <item><description>enables scope validation on the dependency injection container when <see cref="IHostEnvironment.EnvironmentName"/> is 'Development'</description></item>
/// </list>
/// </remarks>
public static HostApplicationBuilder CreateApplicationBuilder() => new HostApplicationBuilder();

I pushed on whether we could just taking a breaking change to HostApplicationBuilder's ctors to make them "empty", but was assured that'd be bad :)

@davidfowl
Copy link
Member

Seems cleaner to have constructors do nothing. I guess it would be bad if you updated hosting on an older version of ASP.NET Core. This API isn't widely used enough yet to be concerned about other scenarios IMHO.

@halter73
Copy link
Member

I agree that there are very few consumers of this API other than ASP.NET Core. Using grep.app, I only found one third-party app creating a HostApplicationBuilder directly. Unfortunately, if we "retcon", any consumers that do exist would almost certainly be broken by the upgrade unless they happened to be setting DisableDefaults = true which seems unlikely.

We could theoretically patch WebApplicationBuilder in .NET 7 to use DisableDefaults = true in order to future-proof it. This would mean copying a lot of code to set up the default settings, config and service into WebApplicationBuilder from Microsoft.Extensions.Hosting. This seems super risky for a patch, but the alternative of completing breaking any consumer of WebApplicationBuilder who updates hosting past version 7 on the ASP.NET Core 7 shared runtime seems worse.

@eerhardt
Copy link
Member Author

eerhardt commented Feb 1, 2023

If we really don't like the existing HostApplicationBuilder constructors, and we really think we need to fix it, there are 2 options I could imagine being feasible:

  1. Make a breaking change to all the HostApplicationBuilder constructors.

    • These would all do what the proposed CreateEmptyApplicationBuilder would do. Using a constructor directly would no longer add appsettings.json config files, or any of the default loggers (like Console).

    • We would still need to add the approved Host.CreateApplicationBuilder(HostApplicationBuilderSettings? settings) overload. This is what "normal" ASP.NET apps (i.e. WebApplication.CreateBuilder) would use.

    • We would need to patch the ASP.NET v7 to still work if the application pulled in the Microsoft.Extensions.Hosting v8+ nupkg. To do this, we could use Reflection to check if Host had a CreateApplicationBuilder overload that took a HostApplicationBuilderSettings parameter. If it did, we are on v8, so call the overload using Reflection. If that method doesn't exist, do what it does today.

    • With this plan, we would also Obsolete HostApplicationBuilderSettings.DisableDefaults property because it would no longer do anything. If you want the "Deafults" - you'd use a Host.CreateApplicationBuilder overload. If you don't want the "Defaults" - you use the constructor. Neither of these APIs would respect this setting.

    • Optionally, we could also obsolete the 2 constructors: parameterless and string[] args. If you want the "empty" behavior, you should specify the HostApplicationBuilderSettings, or else you'd never be able to affect things like EnvironmentName or ContentRootPath.

  2. Cut our losses on the HostApplicationBuilder constructors.

    • This would Obsolete all 3 of the existing HostApplicationBuilder constructors.

    • We would still add the 2 approved methods to Host above.

    • All other functionality remains the same.

      • The constructors still exist, they still do the same behavior as they do today.
      • DisableDefaults still works the same as proposed above - it is respected by the constructors and Host.CreateApplicationBuilder. It is ignored by Host.CreateEmptyApplicationBuilder because there are no defaults to disable.
    • Optionally, if in the future we wanted to revive some/all of the constructors and make the breaking change then, we could do that without having to service ASP.NET 7, since it would be out of support/EOL by then.

I think the plan we approved yesterday is good enough to meet our goals. But if we wanted to make the current APIs better, I would lean towards option 2, and just move away from the constructors all together. This would be consistent with ASP.NET's WebApplicationBuilder, which doesn't expose constructors, but instead uses a static factory method: WebApplication.CreateBuilder().

eerhardt added a commit to eerhardt/runtime that referenced this issue Feb 6, 2023
Introduce a way to create an "empty" HostApplicationBuilder that doesn't bring any optional/unnecessary dependencies into the app's closure. This has the same functional behavior as DisableDefaults=true while allowing for the unused code to be trimmed.

Also add CreateApplicationBuilder(HostApplicationBuilderSettings settings) overload to make the Host factory methods complete.

Fix dotnet#81280
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 6, 2023
eerhardt added a commit that referenced this issue Feb 9, 2023
* Add Host.CreateEmptyApplicationBuilder

Introduce a way to create an "empty" HostApplicationBuilder that doesn't bring any optional/unnecessary dependencies into the app's closure. This has the same functional behavior as DisableDefaults=true while allowing for the unused code to be trimmed.

Also add CreateApplicationBuilder(HostApplicationBuilderSettings settings) overload to make the Host factory methods complete.

Fix #81280
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Hosting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants