-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Server memory pool metrics updates #63032
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
Changes from 2 commits
8f517a2
e8a5921
d991f44
992e4e3
145105c
6040cba
f55f383
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
namespace Microsoft.AspNetCore.Connections; | ||
|
||
/// <summary> | ||
/// Options for configuring a memory pool. | ||
/// </summary> | ||
public sealed class MemoryPoolOptions | ||
JamesNK marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
{ | ||
/// <summary> | ||
/// Gets or sets the owner of the memory pool. This is used for logging and diagnostics purposes. | ||
/// </summary> | ||
public string? Owner { get; set; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T> | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create() -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create(Microsoft.AspNetCore.Connections.MemoryPoolOptions! options) -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.MemoryPoolOptions() -> void | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.get -> string? | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.set -> void |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T> | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create() -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create(Microsoft.AspNetCore.Connections.MemoryPoolOptions! options) -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.MemoryPoolOptions() -> void | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.get -> string? | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.set -> void |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T> | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create() -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create(Microsoft.AspNetCore.Connections.MemoryPoolOptions! options) -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.MemoryPoolOptions() -> void | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.get -> string? | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.set -> void |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T> | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create() -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.IMemoryPoolFactory<T>.Create(Microsoft.AspNetCore.Connections.MemoryPoolOptions! options) -> System.Buffers.MemoryPool<T>! | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.MemoryPoolOptions() -> void | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.get -> string? | ||
Microsoft.AspNetCore.Connections.MemoryPoolOptions.Owner.set -> void |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ public NamedPipeConnectionListener( | |
_log = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes"); | ||
_endpoint = endpoint; | ||
_options = options; | ||
_memoryPool = options.MemoryPoolFactory.Create(); | ||
_memoryPool = options.MemoryPoolFactory.Create(new MemoryPoolOptions { Owner = "kestrel" }); | ||
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. Should these be more specific; 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. Originally I had them be specific, e.g. "namedpipe", but the memory pool is set on the connection that is created, which is then used throughout Kestrel for other reasons. I think it would be confusing if a memory pool that says it is owned by named pipes transport is then used by HTTPs middleware for example. |
||
_listeningToken = _listeningTokenSource.Token; | ||
// Have to create the pool here (instead of DI) because the pool is specific to an endpoint. | ||
_poolPolicy = new NamedPipeServerStreamPoolPolicy(endpoint, options); | ||
|
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.
I was thinking this should be optional so you don't have to new up options every time you create a pool? Are your thoughts that since you shouldn't be creating a new pool very often it's fine to require the small allocation cost?
Uh oh!
There was an error while loading. Please reload this page.
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.
Sure, it could be optional. That would follow what Channel has:
Do you think there should be two create methods on the interface, or one with the options as optional?