Skip to content

Fix #6102 - Intense CPU utilization on page change (#6542) #6658

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

Merged
merged 1 commit into from
Jan 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
37 changes: 34 additions & 3 deletions src/Http/Routing/src/DataSourceDependentCache.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand All @@ -8,7 +8,7 @@

namespace Microsoft.AspNetCore.Routing
{
internal class DataSourceDependentCache<T> where T : class
internal sealed class DataSourceDependentCache<T> : IDisposable where T : class
{
private readonly EndpointDataSource _dataSource;
private readonly Func<IReadOnlyList<Endpoint>, T> _initializeCore;
Expand All @@ -19,13 +19,21 @@ internal class DataSourceDependentCache<T> where T : class
private bool _initialized;
private T _value;

private IDisposable _disposable;
private bool _disposed;

public DataSourceDependentCache(EndpointDataSource dataSource, Func<IReadOnlyList<Endpoint>, T> initialize)
{
if (dataSource == null)
{
throw new ArgumentNullException(nameof(dataSource));
}

if (initialize == null)
{
throw new ArgumentNullException(nameof(initialize));
}

_dataSource = dataSource;
_initializeCore = initialize;

Expand All @@ -51,9 +59,32 @@ private T Initialize()
var changeToken = _dataSource.GetChangeToken();
_value = _initializeCore(_dataSource.Endpoints);

changeToken.RegisterChangeCallback(_initializerWithState, null);
// Don't resubscribe if we're already disposed.
if (_disposed)
{
return _value;
}

_disposable = changeToken.RegisterChangeCallback(_initializerWithState, null);
return _value;
}
}

public void Dispose()
{
lock (_lock)
{
if (!_disposed)
{
_disposable?.Dispose();
_disposable = null;

// Tracking whether we're disposed or not prevents a race-condition
// between disposal and Initialize(). If a change callback fires after
// we dispose, then we don't want to reregister.
_disposed = true;
}
}
}
}
}
7 changes: 6 additions & 1 deletion src/Http/Routing/src/DefaultLinkGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

namespace Microsoft.AspNetCore.Routing
{
internal sealed class DefaultLinkGenerator : LinkGenerator
internal sealed class DefaultLinkGenerator : LinkGenerator, IDisposable
{
private readonly ParameterPolicyFactory _parameterPolicyFactory;
private readonly ObjectPool<UriBuildingContext> _uriBuildingContextPool;
Expand Down Expand Up @@ -364,6 +364,11 @@ public static RouteValueDictionary GetAmbientValues(HttpContext httpContext)
return httpContext?.Features.Get<IRouteValuesFeature>()?.RouteValues;
}

public void Dispose()
{
_cache.Dispose();
}

private static class Log
{
public static class EventIds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static IServiceCollection AddRouting(this IServiceCollection services)
services.TryAddSingleton<MatcherFactory, DfaMatcherFactory>();
services.TryAddTransient<DfaMatcherBuilder>();
services.TryAddSingleton<DfaGraphWriter>();
services.TryAddTransient<DataSourceDependentMatcher.Lifetime>();

// Link generation related services
services.TryAddSingleton<LinkGenerator, DefaultLinkGenerator>();
Expand All @@ -77,7 +78,6 @@ public static IServiceCollection AddRouting(this IServiceCollection services)
//
services.TryAddSingleton<EndpointSelector, DefaultEndpointSelector>();
services.TryAddEnumerable(ServiceDescriptor.Singleton<MatcherPolicy, HttpMethodMatcherPolicy>());

return services;
}

Expand Down
9 changes: 7 additions & 2 deletions src/Http/Routing/src/EndpointNameAddressScheme.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand All @@ -9,7 +9,7 @@

namespace Microsoft.AspNetCore.Routing
{
internal class EndpointNameAddressScheme : IEndpointAddressScheme<string>
internal sealed class EndpointNameAddressScheme : IEndpointAddressScheme<string>, IDisposable
{
private readonly DataSourceDependentCache<Dictionary<string, Endpoint[]>> _cache;

Expand Down Expand Up @@ -103,5 +103,10 @@ string GetEndpointName(Endpoint endpoint)
return endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.EndpointName;
}
}

public void Dispose()
{
_cache.Dispose();
}
}
}
46 changes: 43 additions & 3 deletions src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;

namespace Microsoft.AspNetCore.Routing.Matching
{
internal class DataSourceDependentMatcher : Matcher
internal sealed class DataSourceDependentMatcher : Matcher
{
private readonly Func<MatcherBuilder> _matcherBuilderFactory;
private readonly DataSourceDependentCache<Matcher> _cache;

public DataSourceDependentMatcher(
EndpointDataSource dataSource,
Lifetime lifetime,
Func<MatcherBuilder> matcherBuilderFactory)
{
_matcherBuilderFactory = matcherBuilderFactory;

_cache = new DataSourceDependentCache<Matcher>(dataSource, CreateMatcher);
_cache.EnsureInitialized();

// This will Dispose the cache when the lifetime is disposed, this allows
// the service provider to manage the lifetime of the cache.
lifetime.Cache = _cache;
}

// Used in tests
Expand All @@ -48,5 +52,41 @@ private Matcher CreateMatcher(IReadOnlyList<Endpoint> endpoints)

return builder.Build();
}

// Used to tie the lifetime of a DataSourceDependentCache to the service provider
public sealed class Lifetime : IDisposable
{
private readonly object _lock = new object();
private DataSourceDependentCache<Matcher> _cache;
private bool _disposed;

public DataSourceDependentCache<Matcher> Cache
{
get => _cache;
set
{
lock (_lock)
{
if (_disposed)
{
value?.Dispose();
}

_cache = value;
}
}
}

public void Dispose()
{
lock (_lock)
{
_cache?.Dispose();
_cache = null;

_disposed = true;
}
}
}
}
}
8 changes: 6 additions & 2 deletions src/Http/Routing/src/Matching/DfaMatcherFactory.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -29,7 +29,11 @@ public override Matcher CreateMatcher(EndpointDataSource dataSource)
throw new ArgumentNullException(nameof(dataSource));
}

return new DataSourceDependentMatcher(dataSource, () =>
// Creates a tracking entry in DI to stop listening for change events
// when the services are disposed.
var lifetime = _services.GetRequiredService<DataSourceDependentMatcher.Lifetime>();

return new DataSourceDependentMatcher(dataSource, lifetime, () =>
{
return _services.GetRequiredService<DfaMatcherBuilder>();
});
Expand Down
Loading