This repository was archived by the owner on Dec 18, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 522
Date header performance #220
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
145 changes: 145 additions & 0 deletions
145
src/Microsoft.AspNet.Server.Kestrel/Http/DateHeaderValueManager.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
// 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.Threading; | ||
using Microsoft.AspNet.Server.Kestrel.Infrastructure; | ||
|
||
namespace Microsoft.AspNet.Server.Kestrel.Http | ||
{ | ||
/// <summary> | ||
/// Manages the generation of the date header value. | ||
/// </summary> | ||
public class DateHeaderValueManager : IDisposable | ||
{ | ||
private readonly ISystemClock _systemClock; | ||
private readonly TimeSpan _timeWithoutRequestsUntilIdle; | ||
private readonly TimeSpan _timerInterval; | ||
|
||
private volatile string _dateValue; | ||
private object _timerLocker = new object(); | ||
private bool _isDisposed = false; | ||
private bool _hadRequestsSinceLastTimerTick = false; | ||
private Timer _dateValueTimer; | ||
private DateTimeOffset _lastRequestSeen = DateTimeOffset.MinValue; | ||
|
||
/// <summary> | ||
/// Initializes a new instance of the <see cref="DateHeaderValueManager"/> class. | ||
/// </summary> | ||
public DateHeaderValueManager() | ||
: this( | ||
systemClock: new SystemClock(), | ||
timeWithoutRequestsUntilIdle: TimeSpan.FromSeconds(10), | ||
timerInterval: TimeSpan.FromSeconds(1)) | ||
{ | ||
|
||
} | ||
|
||
// Internal for testing | ||
internal DateHeaderValueManager( | ||
ISystemClock systemClock, | ||
TimeSpan timeWithoutRequestsUntilIdle, | ||
TimeSpan timerInterval) | ||
{ | ||
_systemClock = systemClock; | ||
_timeWithoutRequestsUntilIdle = timeWithoutRequestsUntilIdle; | ||
_timerInterval = timerInterval; | ||
} | ||
|
||
/// <summary> | ||
/// Returns a value representing the current server date/time for use in the HTTP "Date" response header | ||
/// in accordance with http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 | ||
/// </summary> | ||
/// <returns>The value.</returns> | ||
public virtual string GetDateHeaderValue() | ||
{ | ||
PumpTimer(); | ||
|
||
// See https://msdn.microsoft.com/en-us/library/az4se3k1(v=vs.110).aspx#RFC1123 for info on the format | ||
// string used here. | ||
// The null-coalesce here is to protect against returning null after Dispose() is called, at which | ||
// point _dateValue will be null forever after. | ||
return _dateValue ?? _systemClock.UtcNow.ToString(Constants.RFC1123DateFormat); | ||
} | ||
|
||
/// <summary> | ||
/// Releases all resources used by the current instance of <see cref="DateHeaderValueManager"/>. | ||
/// </summary> | ||
public void Dispose() | ||
{ | ||
lock (_timerLocker) | ||
{ | ||
DisposeTimer(); | ||
|
||
_isDisposed = true; | ||
} | ||
} | ||
|
||
private void PumpTimer() | ||
{ | ||
_hadRequestsSinceLastTimerTick = true; | ||
|
||
// If we're already disposed we don't care about starting the timer again. This avoids us having to worry | ||
// about requests in flight during dispose (not that that should actually happen) as those will just get | ||
// SystemClock.UtcNow (aka "the slow way"). | ||
if (!_isDisposed && _dateValueTimer == null) | ||
{ | ||
lock (_timerLocker) | ||
{ | ||
if (!_isDisposed && _dateValueTimer == null) | ||
{ | ||
// Immediately assign the date value and start the timer again. We assign the value immediately | ||
// here as the timer won't fire until the timer interval has passed and we want a value assigned | ||
// inline now to serve requests that occur in the meantime. | ||
_dateValue = _systemClock.UtcNow.ToString(Constants.RFC1123DateFormat); | ||
_dateValueTimer = new Timer(UpdateDateValue, state: null, dueTime: _timerInterval, period: _timerInterval); | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Called by the Timer (background) thread | ||
private void UpdateDateValue(object state) | ||
{ | ||
var now = _systemClock.UtcNow; | ||
|
||
// See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 for required format of Date header | ||
_dateValue = now.ToString(Constants.RFC1123DateFormat); | ||
|
||
if (_hadRequestsSinceLastTimerTick) | ||
{ | ||
// We served requests since the last tick, reset the flag and return as we're still active | ||
_hadRequestsSinceLastTimerTick = false; | ||
_lastRequestSeen = now; | ||
return; | ||
} | ||
|
||
// No requests since the last timer tick, we need to check if we're beyond the idle threshold | ||
var timeSinceLastRequestSeen = now - _lastRequestSeen; | ||
if (timeSinceLastRequestSeen >= _timeWithoutRequestsUntilIdle) | ||
{ | ||
// No requests since idle threshold so stop the timer if it's still running | ||
if (_dateValueTimer != null) | ||
{ | ||
lock (_timerLocker) | ||
{ | ||
if (_dateValueTimer != null) | ||
{ | ||
DisposeTimer(); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
private void DisposeTimer() | ||
{ | ||
if (_dateValueTimer != null) | ||
{ | ||
_dateValueTimer.Dispose(); | ||
_dateValueTimer = null; | ||
_dateValue = null; | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 18 additions & 0 deletions
18
src/Microsoft.AspNet.Server.Kestrel/Infrastructure/ISystemClock.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// 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; | ||
|
||
namespace Microsoft.AspNet.Server.Kestrel.Infrastructure | ||
{ | ||
/// <summary> | ||
/// Abstracts the system clock to facilitate testing. | ||
/// </summary> | ||
internal interface ISystemClock | ||
{ | ||
/// <summary> | ||
/// Retrieves the current system time in UTC. | ||
/// </summary> | ||
DateTimeOffset UtcNow { get; } | ||
} | ||
} |
24 changes: 24 additions & 0 deletions
24
src/Microsoft.AspNet.Server.Kestrel/Infrastructure/SystemClock.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// 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; | ||
|
||
namespace Microsoft.AspNet.Server.Kestrel.Infrastructure | ||
{ | ||
/// <summary> | ||
/// Provides access to the normal system clock. | ||
/// </summary> | ||
internal class SystemClock : ISystemClock | ||
{ | ||
/// <summary> | ||
/// Retrieves the current system time in UTC. | ||
/// </summary> | ||
public DateTimeOffset UtcNow | ||
{ | ||
get | ||
{ | ||
return DateTimeOffset.UtcNow; | ||
} | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
using Microsoft.AspNet.Hosting.Server; | ||
using Microsoft.AspNet.Http.Features; | ||
using Microsoft.AspNet.Server.Features; | ||
using Microsoft.AspNet.Server.Kestrel.Http; | ||
using Microsoft.Dnx.Runtime; | ||
using Microsoft.Framework.Configuration; | ||
using Microsoft.Framework.Logging; | ||
|
@@ -53,9 +54,16 @@ public IDisposable Start(IFeatureCollection serverFeatures, Func<IFeatureCollect | |
try | ||
{ | ||
var information = (KestrelServerInformation)serverFeatures.Get<IKestrelServerInformation>(); | ||
var engine = new KestrelEngine(_libraryManager, new ServiceContext { AppShutdown = _appShutdownService, Log = new KestrelTrace(_logger) }); | ||
var dateHeaderValueManager = new DateHeaderValueManager(); | ||
var engine = new KestrelEngine(_libraryManager, new ServiceContext | ||
{ | ||
AppShutdown = _appShutdownService, | ||
Log = new KestrelTrace(_logger), | ||
DateHeaderValueManager = dateHeaderValueManager | ||
}); | ||
|
||
disposables.Push(engine); | ||
disposables.Push(dateHeaderValueManager); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, my There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's messy, but this ensures that "he who creates it disposes it". We should look at this all up later. |
||
|
||
if (information.ThreadCount < 0) | ||
{ | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
test/Microsoft.AspNet.Server.KestrelTests/DateHeaderValueManagerTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
// 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.Threading.Tasks; | ||
using Microsoft.AspNet.Server.Kestrel.Http; | ||
using Microsoft.AspNet.Server.Kestrel.Infrastructure; | ||
using Microsoft.AspNet.Server.KestrelTests.TestHelpers; | ||
using Xunit; | ||
|
||
namespace Microsoft.AspNet.Server.KestrelTests | ||
{ | ||
public class DateHeaderValueManagerTests | ||
{ | ||
[Fact] | ||
public void GetDateHeaderValue_ReturnsDateValueInRFC1123Format() | ||
{ | ||
var now = DateTimeOffset.UtcNow; | ||
var systemClock = new MockSystemClock | ||
{ | ||
UtcNow = now | ||
}; | ||
var timeWithoutRequestsUntilIdle = TimeSpan.FromSeconds(1); | ||
var timerInterval = TimeSpan.FromSeconds(10); | ||
var dateHeaderValueManager = new DateHeaderValueManager(systemClock, timeWithoutRequestsUntilIdle, timerInterval); | ||
string result; | ||
|
||
try | ||
{ | ||
result = dateHeaderValueManager.GetDateHeaderValue(); | ||
} | ||
finally | ||
{ | ||
dateHeaderValueManager.Dispose(); | ||
} | ||
|
||
Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result); | ||
} | ||
|
||
[Fact] | ||
public void GetDateHeaderValue_ReturnsCachedValueBetweenTimerTicks() | ||
{ | ||
var now = DateTimeOffset.UtcNow; | ||
var future = now.AddSeconds(10); | ||
var systemClock = new MockSystemClock | ||
{ | ||
UtcNow = now | ||
}; | ||
var timeWithoutRequestsUntilIdle = TimeSpan.FromSeconds(1); | ||
var timerInterval = TimeSpan.FromSeconds(10); | ||
var dateHeaderValueManager = new DateHeaderValueManager(systemClock, timeWithoutRequestsUntilIdle, timerInterval); | ||
string result1; | ||
string result2; | ||
|
||
try | ||
{ | ||
result1 = dateHeaderValueManager.GetDateHeaderValue(); | ||
systemClock.UtcNow = future; | ||
result2 = dateHeaderValueManager.GetDateHeaderValue(); | ||
} | ||
finally | ||
{ | ||
dateHeaderValueManager.Dispose(); | ||
} | ||
|
||
Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1); | ||
Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result2); | ||
Assert.Equal(1, systemClock.UtcNowCalled); | ||
} | ||
|
||
[Fact] | ||
public async Task GetDateHeaderValue_ReturnsUpdatedValueAfterIdle() | ||
{ | ||
var now = DateTimeOffset.UtcNow; | ||
var future = now.AddSeconds(10); | ||
var systemClock = new MockSystemClock | ||
{ | ||
UtcNow = now | ||
}; | ||
var timeWithoutRequestsUntilIdle = TimeSpan.FromMilliseconds(50); | ||
var timerInterval = TimeSpan.FromMilliseconds(10); | ||
var dateHeaderValueManager = new DateHeaderValueManager(systemClock, timeWithoutRequestsUntilIdle, timerInterval); | ||
string result1; | ||
string result2; | ||
|
||
try | ||
{ | ||
result1 = dateHeaderValueManager.GetDateHeaderValue(); | ||
systemClock.UtcNow = future; | ||
// Wait for twice the idle timeout to ensure the timer is stopped | ||
await Task.Delay(timeWithoutRequestsUntilIdle.Add(timeWithoutRequestsUntilIdle)); | ||
result2 = dateHeaderValueManager.GetDateHeaderValue(); | ||
} | ||
finally | ||
{ | ||
dateHeaderValueManager.Dispose(); | ||
} | ||
|
||
Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1); | ||
Assert.Equal(future.ToString(Constants.RFC1123DateFormat), result2); | ||
Assert.True(systemClock.UtcNowCalled >= 2); | ||
} | ||
|
||
[Fact] | ||
public void GetDateHeaderValue_ReturnsDateValueAfterDisposed() | ||
{ | ||
var now = DateTimeOffset.UtcNow; | ||
var future = now.AddSeconds(10); | ||
var systemClock = new MockSystemClock | ||
{ | ||
UtcNow = now | ||
}; | ||
var timeWithoutRequestsUntilIdle = TimeSpan.FromSeconds(1); | ||
var timerInterval = TimeSpan.FromSeconds(10); | ||
var dateHeaderValueManager = new DateHeaderValueManager(systemClock, timeWithoutRequestsUntilIdle, timerInterval); | ||
|
||
var result1 = dateHeaderValueManager.GetDateHeaderValue(); | ||
dateHeaderValueManager.Dispose(); | ||
systemClock.UtcNow = future; | ||
var result2 = dateHeaderValueManager.GetDateHeaderValue(); | ||
|
||
Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1); | ||
Assert.Equal(future.ToString(Constants.RFC1123DateFormat), result2); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Provide default "new" in ServiceContect ctor instead of this code path?
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.
This is a little messy, as whoever creates the
DateHeaderManager
should be disposing it, andServiceContext
isn't disposable, and everything else derives from it, and you don't want them to destroy the default instance ofDateHeaderManager
in their base. We might have to fix this more generically later for things likeMemoryPool
as well that hang off ofServiceContext
.