Skip to content

Commit 3e5b37f

Browse files
committed
Fix #6102 - Intense CPU utilization on page change (#6542)
* Fix #6102 - Intense CPU utilization on page change The issue here was that every time a Razor Page changed, we would subscribe an additional time to the endpoint change notifications. This means that if you tweaked a page 30 times, we would update the address table 31 times when you save the file. If you were doing a lot of editing then this would grow to a really large amount of computation. The fix is to use DataSourceDependentCache, which is an existing utility type we developed for this purpose. I'm not sure why it wasn't being used for this already. We're already using DataSourceDependentCache in a bunch of other places, and it's well tested. I also tweaked the stucture of this code to be more similar to EndpointNameAddressScheme. This involved some test changes that all seemed like good cleanup. The way this was being tested was a little wonky. (cherry picked from commit a5658a8)
1 parent cd308e7 commit 3e5b37f

10 files changed

+276
-151
lines changed

src/Http/Routing/src/DataSourceDependentCache.cs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -8,7 +8,7 @@
88

99
namespace Microsoft.AspNetCore.Routing
1010
{
11-
internal class DataSourceDependentCache<T> where T : class
11+
internal sealed class DataSourceDependentCache<T> : IDisposable where T : class
1212
{
1313
private readonly EndpointDataSource _dataSource;
1414
private readonly Func<IReadOnlyList<Endpoint>, T> _initializeCore;
@@ -19,13 +19,21 @@ internal class DataSourceDependentCache<T> where T : class
1919
private bool _initialized;
2020
private T _value;
2121

22+
private IDisposable _disposable;
23+
private bool _disposed;
24+
2225
public DataSourceDependentCache(EndpointDataSource dataSource, Func<IReadOnlyList<Endpoint>, T> initialize)
2326
{
2427
if (dataSource == null)
2528
{
2629
throw new ArgumentNullException(nameof(dataSource));
2730
}
2831

32+
if (initialize == null)
33+
{
34+
throw new ArgumentNullException(nameof(initialize));
35+
}
36+
2937
_dataSource = dataSource;
3038
_initializeCore = initialize;
3139

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

54-
changeToken.RegisterChangeCallback(_initializerWithState, null);
62+
// Don't resubscribe if we're already disposed.
63+
if (_disposed)
64+
{
65+
return _value;
66+
}
67+
68+
_disposable = changeToken.RegisterChangeCallback(_initializerWithState, null);
5569
return _value;
5670
}
5771
}
72+
73+
public void Dispose()
74+
{
75+
lock (_lock)
76+
{
77+
if (!_disposed)
78+
{
79+
_disposable?.Dispose();
80+
_disposable = null;
81+
82+
// Tracking whether we're disposed or not prevents a race-condition
83+
// between disposal and Initialize(). If a change callback fires after
84+
// we dispose, then we don't want to reregister.
85+
_disposed = true;
86+
}
87+
}
88+
}
5889
}
5990
}

src/Http/Routing/src/DefaultLinkGenerator.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
namespace Microsoft.AspNetCore.Routing
2121
{
22-
internal sealed class DefaultLinkGenerator : LinkGenerator
22+
internal sealed class DefaultLinkGenerator : LinkGenerator, IDisposable
2323
{
2424
private readonly ParameterPolicyFactory _parameterPolicyFactory;
2525
private readonly ObjectPool<UriBuildingContext> _uriBuildingContextPool;
@@ -364,6 +364,11 @@ public static RouteValueDictionary GetAmbientValues(HttpContext httpContext)
364364
return httpContext?.Features.Get<IRouteValuesFeature>()?.RouteValues;
365365
}
366366

367+
public void Dispose()
368+
{
369+
_cache.Dispose();
370+
}
371+
367372
private static class Log
368373
{
369374
public static class EventIds

src/Http/Routing/src/DependencyInjection/RoutingServiceCollectionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public static IServiceCollection AddRouting(this IServiceCollection services)
6666
services.TryAddSingleton<MatcherFactory, DfaMatcherFactory>();
6767
services.TryAddTransient<DfaMatcherBuilder>();
6868
services.TryAddSingleton<DfaGraphWriter>();
69+
services.TryAddTransient<DataSourceDependentMatcher.Lifetime>();
6970

7071
// Link generation related services
7172
services.TryAddSingleton<LinkGenerator, DefaultLinkGenerator>();
@@ -77,7 +78,6 @@ public static IServiceCollection AddRouting(this IServiceCollection services)
7778
//
7879
services.TryAddSingleton<EndpointSelector, DefaultEndpointSelector>();
7980
services.TryAddEnumerable(ServiceDescriptor.Singleton<MatcherPolicy, HttpMethodMatcherPolicy>());
80-
8181
return services;
8282
}
8383

src/Http/Routing/src/EndpointNameAddressScheme.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -9,7 +9,7 @@
99

1010
namespace Microsoft.AspNetCore.Routing
1111
{
12-
internal class EndpointNameAddressScheme : IEndpointAddressScheme<string>
12+
internal sealed class EndpointNameAddressScheme : IEndpointAddressScheme<string>, IDisposable
1313
{
1414
private readonly DataSourceDependentCache<Dictionary<string, Endpoint[]>> _cache;
1515

@@ -103,5 +103,10 @@ string GetEndpointName(Endpoint endpoint)
103103
return endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.EndpointName;
104104
}
105105
}
106+
107+
public void Dispose()
108+
{
109+
_cache.Dispose();
110+
}
106111
}
107112
}

src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,31 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
55
using System.Collections.Generic;
66
using System.Threading.Tasks;
77
using Microsoft.AspNetCore.Http;
8-
using Microsoft.AspNetCore.Http.Features;
98

109
namespace Microsoft.AspNetCore.Routing.Matching
1110
{
12-
internal class DataSourceDependentMatcher : Matcher
11+
internal sealed class DataSourceDependentMatcher : Matcher
1312
{
1413
private readonly Func<MatcherBuilder> _matcherBuilderFactory;
1514
private readonly DataSourceDependentCache<Matcher> _cache;
1615

1716
public DataSourceDependentMatcher(
1817
EndpointDataSource dataSource,
18+
Lifetime lifetime,
1919
Func<MatcherBuilder> matcherBuilderFactory)
2020
{
2121
_matcherBuilderFactory = matcherBuilderFactory;
2222

2323
_cache = new DataSourceDependentCache<Matcher>(dataSource, CreateMatcher);
2424
_cache.EnsureInitialized();
25+
26+
// This will Dispose the cache when the lifetime is disposed, this allows
27+
// the service provider to manage the lifetime of the cache.
28+
lifetime.Cache = _cache;
2529
}
2630

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

4953
return builder.Build();
5054
}
55+
56+
// Used to tie the lifetime of a DataSourceDependentCache to the service provider
57+
public sealed class Lifetime : IDisposable
58+
{
59+
private readonly object _lock = new object();
60+
private DataSourceDependentCache<Matcher> _cache;
61+
private bool _disposed;
62+
63+
public DataSourceDependentCache<Matcher> Cache
64+
{
65+
get => _cache;
66+
set
67+
{
68+
lock (_lock)
69+
{
70+
if (_disposed)
71+
{
72+
value?.Dispose();
73+
}
74+
75+
_cache = value;
76+
}
77+
}
78+
}
79+
80+
public void Dispose()
81+
{
82+
lock (_lock)
83+
{
84+
_cache?.Dispose();
85+
_cache = null;
86+
87+
_disposed = true;
88+
}
89+
}
90+
}
5191
}
5292
}

src/Http/Routing/src/Matching/DfaMatcherFactory.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -29,7 +29,11 @@ public override Matcher CreateMatcher(EndpointDataSource dataSource)
2929
throw new ArgumentNullException(nameof(dataSource));
3030
}
3131

32-
return new DataSourceDependentMatcher(dataSource, () =>
32+
// Creates a tracking entry in DI to stop listening for change events
33+
// when the services are disposed.
34+
var lifetime = _services.GetRequiredService<DataSourceDependentMatcher.Lifetime>();
35+
36+
return new DataSourceDependentMatcher(dataSource, lifetime, () =>
3337
{
3438
return _services.GetRequiredService<DfaMatcherBuilder>();
3539
});

0 commit comments

Comments
 (0)