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

Optimize modules loading. #2972

Merged
merged 2 commits into from
Jan 7, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Hosting;

namespace OrchardCore.Modules
Expand Down Expand Up @@ -48,12 +49,17 @@ private void EnsureInitialized()

private IEnumerable<Module> GetModules()
{
var modules = new List<Module>();
var modules = new ConcurrentBag<Module>();
modules.Add(new Module(_environment.ApplicationName, true));

foreach (var provider in _moduleNamesProviders)
{
modules.AddRange(provider.GetModuleNames().Select(name => new Module(name, name == _environment.ApplicationName)));
var names = provider.GetModuleNames().Distinct();

Parallel.ForEach(names, new ParallelOptions { MaxDegreeOfParallelism = 8 }, (name) =>
{
modules.Add(new Module(name, name == _environment.ApplicationName));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering... should name and application name be case insensitive? :/

Copy link
Member

@Jetski5822 Jetski5822 Jan 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think this whole section is kinda wrong... shouldnt it be more like this...

	modules.AddRange(_moduleNamesProviders
		.SelectMany(x => x.GetModuleNames())
		.Distinct()
		.AsParallel()
		.Select(name => new Module(name, name == _environment.ApplicationName)));

});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to stick using Linq, but do AsParrellel() ?

}

return modules;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ namespace OrchardCore.Environment.Shell.Builders
{
public class ShellContainerFactory : IShellContainerFactory
{
private readonly IFeatureInfo _applicationFeature;
private IFeatureInfo _applicationFeature;

private readonly IHostingEnvironment _hostingEnvironment;
private readonly IExtensionManager _extensionManager;
private readonly IServiceProvider _serviceProvider;
private readonly ILogger _logger;
private readonly ILoggerFactory _loggerFactory;
Expand All @@ -27,9 +30,8 @@ public ShellContainerFactory(
ILogger<ShellContainerFactory> logger,
IServiceCollection applicationServices)
{
_applicationFeature = extensionManager.GetFeatures().FirstOrDefault(
f => f.Id == hostingEnvironment.ApplicationName);

_hostingEnvironment = hostingEnvironment;
_extensionManager = extensionManager;
_applicationServices = applicationServices;
_serviceProvider = serviceProvider;
_loggerFactory = loggerFactory;
Expand Down Expand Up @@ -84,6 +86,10 @@ public IServiceProvider CreateContainer(ShellSettings settings, ShellBlueprint b
// OrderBy performs a stable sort so order is preserved among equal Order values.
startups = startups.OrderBy(s => s.Order);

// To not trigger features loading before it is normally done by 'ShellHost',
// init here the application feature in place of doing it in the constructor.
EnsureApplicationFeature();

// Let any module add custom service descriptors to the tenant
foreach (var startup in startups)
{
Expand All @@ -97,7 +103,7 @@ public IServiceProvider CreateContainer(ShellSettings settings, ShellBlueprint b
}

(moduleServiceProvider as IDisposable).Dispose();

var shellServiceProvider = tenantServiceCollection.BuildServiceProvider(true);

// Register all DIed types in ITypeFeatureProvider
Expand All @@ -124,5 +130,20 @@ public IServiceProvider CreateContainer(ShellSettings settings, ShellBlueprint b

return shellServiceProvider;
}

private void EnsureApplicationFeature()
{
if (_applicationFeature == null)
{
lock (this)
{
if (_applicationFeature == null)
{
_applicationFeature = _extensionManager.GetFeatures()
.FirstOrDefault(f => f.Id == _hostingEnvironment.ApplicationName);
}
}
}
}
}
}