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.

Sign up for GitHub

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

Add SystemTextJsonSerializer.SupportsFastPath method #154

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+43 −20
Open

Add SystemTextJsonSerializer.SupportsFastPath method #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 23 additions & 0 deletions src/Elastic.Transport/Components/Serialization/SystemTextJsonSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,29 @@ public override Task SerializeAsync<T>(T data, Stream stream,

#endregion Serializer

/// <summary>
/// Override to (dis-)allow fast-path (de-)serialization for specific types.
/// </summary>
/// <param name="type">The <see cref="Type"/> that is being (de-)serialized.</param>
/// <returns>
/// <see langword="true"/> if the given <paramref name="type"/> supports fast-path (de-)serialization or
/// <see langword="false"/>, if not.
/// </returns>
/// <remarks>
/// <para>
/// Most extension methods in <see cref="Extensions.TransportSerializerExtensions"/> will prefer fast-path (de-)serialization, when
/// used with <see cref="SystemTextJsonSerializer"/> based serializer implementations.
/// Fast-path (de-)serialization bypasses the <see cref="Deserialize"/>, <see cref="Deserialize{T}"/>, <see cref="Serialize{T}"/>
/// methods and directly uses the <see cref="JsonSerializer"/> API instead.
/// </para>
/// <para>
/// In some cases, when the concrete <see cref="SystemTextJsonSerializer"/> based serializer implementation overrides one or more of
/// the named methods, the default fast-path behavior might be undesired. The <see cref="SupportsFastPath"/> method can be used to
Copy link
Member

Choose a reason for hiding this comment

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

Can you expand why the fast path might be undesirable?

I grasp the use case for not prefering the fast path.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the user overrides one of these methods, there is probably a reason for doing so.

Like mentioned, fast path will bypass the Deserialize/Serialize method invocation, which in conclusion will as well bypass execution of the user code (which is why fast path is undesirable in these cases, otherwise there wouldn't have been a good reason for overriding Deserialize/Serialize in the first place).

Is that unclear from the doc comment?

Copy link
Member

Choose a reason for hiding this comment

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

Comments make more sense in the morning then late at night :)

Copy link
Member Author

Choose a reason for hiding this comment

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

True :D Thanks for the review! I will try to rephrase it a bit before merging to make it more clear.

/// completely disable fast-path (de-)serialization or selectively use it for specific types only.
/// </para>
/// </remarks>
protected internal virtual bool SupportsFastPath(Type type) => true;

/// <summary>
/// Returns the <see cref="JsonSerializerOptions"/> for this serializer, based on the given <paramref name="formatting"/>.
/// </summary>
Expand Down
36 changes: 18 additions & 18 deletions src/Elastic.Transport/Components/Serialization/TransportSerializerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static byte[] SerializeToBytes<T>(
SerializationFormatting formatting = SerializationFormatting.None
)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations.
return JsonSerializer.SerializeToUtf8Bytes(data, stjSerializer.GetJsonSerializerOptions(formatting));
Expand Down Expand Up @@ -92,7 +92,7 @@ public static byte[] SerializeToBytes(
SerializationFormatting formatting = SerializationFormatting.None
)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations.
return JsonSerializer.SerializeToUtf8Bytes(data, type, stjSerializer.GetJsonSerializerOptions(formatting));
Expand Down Expand Up @@ -135,7 +135,7 @@ public static string SerializeToString<T>(
SerializationFormatting formatting = SerializationFormatting.None
)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// serialize straight into string.
Expand Down Expand Up @@ -183,7 +183,7 @@ public static string SerializeToString(
SerializationFormatting formatting = SerializationFormatting.None
)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// serialize straight into string.
Expand Down Expand Up @@ -234,7 +234,7 @@ public static void Serialize<T>(
MemoryStreamFactory? memoryStreamFactory,
SerializationFormatting formatting = SerializationFormatting.None)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// serialize straight into the writer.
Expand Down Expand Up @@ -294,7 +294,7 @@ public static void Serialize(
MemoryStreamFactory? memoryStreamFactory,
SerializationFormatting formatting = SerializationFormatting.None)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// serialize straight into the writer.
Expand Down Expand Up @@ -332,7 +332,7 @@ public static void Serialize(
string input,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -362,7 +362,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -391,7 +391,7 @@ public static void Serialize(
ReadOnlySpan<byte> span,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -421,7 +421,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -450,7 +450,7 @@ public static void Serialize(
ReadOnlySpan<char> span,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -480,7 +480,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the span.
Expand Down Expand Up @@ -509,7 +509,7 @@ public static void Serialize(
ref Utf8JsonReader reader,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the reader.
Expand Down Expand Up @@ -546,7 +546,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the reader.
Expand Down Expand Up @@ -582,7 +582,7 @@ public static void Serialize(
JsonNode node,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the node.
Expand Down Expand Up @@ -617,7 +617,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the node.
Expand Down Expand Up @@ -651,7 +651,7 @@ public static void Serialize(
JsonElement node,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(typeof(T)))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the node.
Expand Down Expand Up @@ -686,7 +686,7 @@ public static void Serialize(
Type type,
MemoryStreamFactory? memoryStreamFactory = null)
{
if (serializer is SystemTextJsonSerializer stjSerializer)
if (serializer is SystemTextJsonSerializer stjSerializer && stjSerializer.SupportsFastPath(type))
{
// When the serializer derives from `SystemTextJsonSerializer` we can avoid unnecessary allocations and
// deserialize straight from the node.
Expand Down
4 changes: 2 additions & 2 deletions src/Elastic.Transport/Requests/IUrlParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ namespace Elastic.Transport;
/// <summary> Implementers define an object that can be serialized as a query string parameter </summary>
public interface IUrlParameter
{
/// <summary> Get the a string representation using <paramref name="settings"/> </summary>
string GetString(ITransportConfiguration settings);
/// <summary> Get the string representation using <paramref name="settings"/> </summary>
public string GetString(ITransportConfiguration settings);
}
Loading