diff --git a/sdk/core/Azure.Core/perf/Azure.Core.Perf.csproj b/sdk/core/Azure.Core/perf/Azure.Core.Perf.csproj index 035fa2515d41..c3bd504fdbaa 100644 --- a/sdk/core/Azure.Core/perf/Azure.Core.Perf.csproj +++ b/sdk/core/Azure.Core/perf/Azure.Core.Perf.csproj @@ -22,13 +22,13 @@ - + Always - + Always - + Always diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelContent/LargeModel.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/LargeModel.cs new file mode 100644 index 000000000000..692b20efa53a --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/LargeModel.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Tests.Public.ResourceManager.Resources; + +namespace Azure.Core.Perf.RequestContents.ModelContent +{ + public class LargeModel : ModelContentBenchmark + { + protected override string JsonFileName => "ResourceProviderData.json"; + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelContent/ModelContentBenchmark.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/ModelContentBenchmark.cs new file mode 100644 index 000000000000..ac3a6acc3819 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/ModelContentBenchmark.cs @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Serialization; + +namespace Azure.Core.Perf.RequestContents.ModelContent +{ + public abstract class ModelContentBenchmark : RequestContentBenchmark> where T : class, IModelSerializable + { + protected override RequestContent CreateRequestContent() + { + return RequestContent.Create(_model); + } + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelContent/SmallModel.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/SmallModel.cs new file mode 100644 index 000000000000..9165fa610032 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelContent/SmallModel.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Tests.Public.ResourceManager.Compute; + +namespace Azure.Core.Perf.RequestContents.ModelContent +{ + public class SmallModel : ModelContentBenchmark + { + protected override string JsonFileName => "AvailabilitySetData.json"; + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/LargeModel.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/LargeModel.cs new file mode 100644 index 000000000000..81bf67044a41 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/LargeModel.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Tests.Public.ResourceManager.Resources; + +namespace Azure.Core.Perf.RequestContents.ModelJsonContent +{ + public class LargeModel : ModelJsonContentBenchmark + { + protected override string JsonFileName => "ResourceProviderData.json"; + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/ModelJsonContentBenchmark.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/ModelJsonContentBenchmark.cs new file mode 100644 index 000000000000..c44d7fb23a22 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/ModelJsonContentBenchmark.cs @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Serialization; + +namespace Azure.Core.Perf.RequestContents.ModelJsonContent +{ + public abstract class ModelJsonContentBenchmark : RequestContentBenchmark> where T : class, IModelJsonSerializable + { + protected override RequestContent CreateRequestContent() + { + return RequestContent.Create(_model); + } + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/SmallModel.cs b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/SmallModel.cs new file mode 100644 index 000000000000..47b32034d476 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/ModelJsonContent/SmallModel.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Azure.Core.Tests.Public.ResourceManager.Compute; + +namespace Azure.Core.Perf.RequestContents.ModelJsonContent +{ + public class SmallModel : ModelJsonContentBenchmark + { + protected override string JsonFileName => "AvailabilitySetData.json"; + } +} diff --git a/sdk/core/Azure.Core/perf/RequestContents/RequestContentBenchmark.cs b/sdk/core/Azure.Core/perf/RequestContents/RequestContentBenchmark.cs new file mode 100644 index 000000000000..38f2b2f43a00 --- /dev/null +++ b/sdk/core/Azure.Core/perf/RequestContents/RequestContentBenchmark.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.IO; +using System.Reflection; +using Azure.Core.Serialization; +using BenchmarkDotNet.Attributes; + +namespace Azure.Core.Perf.RequestContents +{ + public abstract class RequestContentBenchmark where T : class + { + protected abstract string JsonFileName { get; } + protected abstract RequestContent CreateRequestContent(); + + protected T _model; + private RequestContent _serializedContent; + private MemoryStream _stream; + + [GlobalSetup] + public void GlobalSetup() + { + string json = File.ReadAllText(Path.Combine(Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName, "TestData", JsonFileName)); + Type modelType = typeof(T).GetGenericArguments()[0]; + _model = ModelSerializer.Deserialize(BinaryData.FromString(json), modelType) as T; + _serializedContent = CreateRequestContent(); + _serializedContent.TryComputeLength(out long length); + _stream = new MemoryStream((int)length); + } + + [GlobalCleanup] + public void GlobalCleanup() + { + _serializedContent.Dispose(); + _serializedContent = null; + } + + [Benchmark] + public void Construct() + { + using RequestContent content = CreateRequestContent(); + } + + [Benchmark] + public long TryComputeLength() + { + using RequestContent content = CreateRequestContent(); + content.TryComputeLength(out long length); + return length; + } + + [Benchmark] + public void WriteTo() + { + _serializedContent.WriteTo(_stream, default); + _stream.Position = 0; + } + } +} diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/AvailabilitySetDataBenchmark.cs b/sdk/core/Azure.Core/perf/Serializations/AvailabilitySetDataModel.cs similarity index 87% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/AvailabilitySetDataBenchmark.cs rename to sdk/core/Azure.Core/perf/Serializations/AvailabilitySetDataModel.cs index 735ac15cc711..007454eb7294 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/AvailabilitySetDataBenchmark.cs +++ b/sdk/core/Azure.Core/perf/Serializations/AvailabilitySetDataModel.cs @@ -4,9 +4,9 @@ using System.Text.Json; using Azure.Core.Tests.Public.ResourceManager.Compute; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { - public class AvailabilitySetDataBenchmark : JsonSerializationBenchmark + public class AvailabilitySetDataModel : JsonBenchmark { protected override AvailabilitySetData Deserialize(JsonElement jsonElement) { diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/SerializationBenchmarkConfig.cs b/sdk/core/Azure.Core/perf/Serializations/BenchmarkConfig.cs similarity index 74% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/SerializationBenchmarkConfig.cs rename to sdk/core/Azure.Core/perf/Serializations/BenchmarkConfig.cs index 36901ef62427..4468174c3df3 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/SerializationBenchmarkConfig.cs +++ b/sdk/core/Azure.Core/perf/Serializations/BenchmarkConfig.cs @@ -6,11 +6,11 @@ using BenchmarkDotNet.Reports; using Perfolizer.Horology; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { - internal class SerializationBenchmarkConfig : ManualConfig + internal class BenchmarkConfig : ManualConfig { - public SerializationBenchmarkConfig() + public BenchmarkConfig() { SummaryStyle = SummaryStyle.Default .WithTimeUnit(TimeUnit.Microsecond) diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/BimodalRepro.cs b/sdk/core/Azure.Core/perf/Serializations/BimodalRepro.cs similarity index 89% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/BimodalRepro.cs rename to sdk/core/Azure.Core/perf/Serializations/BimodalRepro.cs index 3a56b1933bdb..4dc3e56ba61b 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/BimodalRepro.cs +++ b/sdk/core/Azure.Core/perf/Serializations/BimodalRepro.cs @@ -3,7 +3,7 @@ using BenchmarkDotNet.Attributes; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { public class BimodalRepro { diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/JsonSerializationBenchmark.cs b/sdk/core/Azure.Core/perf/Serializations/JsonBenchmark.cs similarity index 88% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/JsonSerializationBenchmark.cs rename to sdk/core/Azure.Core/perf/Serializations/JsonBenchmark.cs index 3d1ef1f1baef..a25d8bd2f84c 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/JsonSerializationBenchmark.cs +++ b/sdk/core/Azure.Core/perf/Serializations/JsonBenchmark.cs @@ -2,8 +2,6 @@ // Licensed under the MIT License. using System; -using System.Buffers; -using System.Collections.Generic; using System.IO; using System.Reflection; using System.Text; @@ -13,10 +11,10 @@ using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Configs; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)] - public abstract class JsonSerializationBenchmark where T : class, IModelJsonSerializable + public abstract class JsonBenchmark where T : class, IModelJsonSerializable { private string _json; protected T _model; @@ -24,7 +22,6 @@ public abstract class JsonSerializationBenchmark where T : class, IModelJsonS protected ModelSerializerOptions _options; private BinaryData _data; private SequenceWriter _content; - private ReadOnlySequence _sequence; private JsonDocument _jsonDocument; protected abstract T Deserialize(JsonElement jsonElement); @@ -40,7 +37,7 @@ public abstract class JsonSerializationBenchmark where T : class, IModelJsonS [GlobalSetup] public void SetUp() { - _json = File.ReadAllText(Path.Combine(Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName, "SerializationBenchmark", "TestData", JsonFileName)); + _json = File.ReadAllText(Path.Combine(Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName, "TestData", JsonFileName)); _data = BinaryData.FromString(_json); _model = ModelSerializer.Deserialize(_data); _response = new MockResponse(200); @@ -50,7 +47,6 @@ public void SetUp() using Utf8JsonWriter writer = new Utf8JsonWriter(_content); _model.Serialize(writer, new ModelSerializerOptions()); writer.Flush(); - _sequence = _content.GetReadOnlySequence(); _jsonDocument = JsonDocument.Parse(_json); } @@ -176,20 +172,6 @@ public T Deserialize_Utf8JsonReaderFromBinaryData() return _model.Deserialize(ref reader, _options); } - [Benchmark] - [BenchmarkCategory("JsonDocument")] - public ReadOnlySequence GetSequence() - { - return _content.GetReadOnlySequence(); - } - - [Benchmark] - [BenchmarkCategory("JsonDocument")] - public void JsonDocumentFromSequence() - { - using var doc = JsonDocument.Parse(_sequence); - } - [Benchmark] [BenchmarkCategory("JsonDocument")] public void JsonDocumentFromReader() diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/ModelXmlBenchmark.cs b/sdk/core/Azure.Core/perf/Serializations/ModelXmlModel.cs similarity index 87% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/ModelXmlBenchmark.cs rename to sdk/core/Azure.Core/perf/Serializations/ModelXmlModel.cs index 9b6dcbc82d79..9ca689ce948a 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/ModelXmlBenchmark.cs +++ b/sdk/core/Azure.Core/perf/Serializations/ModelXmlModel.cs @@ -1,14 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System; using System.Xml; using System.Xml.Linq; using Azure.Core.Tests.Public.ModelSerializationTests.Models; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { - public class ModelXmlBenchmark : XmlSerializationBenchmark + public class ModelXmlModel : XmlBenchmark { protected override string XmlFileName => "ModelXml.xml"; diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/ResourceProviderDataBenchmark.cs b/sdk/core/Azure.Core/perf/Serializations/ResourceProviderDataModel.cs similarity index 82% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/ResourceProviderDataBenchmark.cs rename to sdk/core/Azure.Core/perf/Serializations/ResourceProviderDataModel.cs index c28d9ba1e042..229a4594c024 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/ResourceProviderDataBenchmark.cs +++ b/sdk/core/Azure.Core/perf/Serializations/ResourceProviderDataModel.cs @@ -1,15 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System; using System.Text.Json; using Azure.Core.Tests.Public.ResourceManager.Resources; using BenchmarkDotNet.Attributes; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { - [Config(typeof(SerializationBenchmarkConfig))] - public class ResourceProviderDataBenchmark : JsonSerializationBenchmark + [Config(typeof(BenchmarkConfig))] + public class ResourceProviderDataModel : JsonBenchmark { protected override string JsonFileName => "ResourceProviderData.json"; diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/XmlSerializationBenchmark.cs b/sdk/core/Azure.Core/perf/Serializations/XmlBenchmark.cs similarity index 94% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/XmlSerializationBenchmark.cs rename to sdk/core/Azure.Core/perf/Serializations/XmlBenchmark.cs index a9ff0b6f3db6..5b0803983119 100644 --- a/sdk/core/Azure.Core/perf/SerializationBenchmark/XmlSerializationBenchmark.cs +++ b/sdk/core/Azure.Core/perf/Serializations/XmlBenchmark.cs @@ -12,10 +12,10 @@ using BenchmarkDotNet.Attributes; using BenchmarkDotNet.Configs; -namespace Azure.Core.Perf +namespace Azure.Core.Perf.Serializations { [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)] - public abstract class XmlSerializationBenchmark where T : class, IModelSerializable + public abstract class XmlBenchmark where T : class, IModelSerializable { private string _xml; protected T _model; @@ -37,7 +37,7 @@ public abstract class XmlSerializationBenchmark where T : class, IModelSerial [GlobalSetup] public void SetUp() { - _xml = File.ReadAllText(Path.Combine(Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName, "SerializationBenchmark", "TestData", XmlFileName)); + _xml = File.ReadAllText(Path.Combine(Directory.GetParent(Assembly.GetExecutingAssembly().Location).FullName, "TestData", XmlFileName)); _data = BinaryData.FromString(_xml); _model = ModelSerializer.Deserialize(_data); _response = new MockResponse(200); diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/AvailabilitySetData.json b/sdk/core/Azure.Core/perf/TestData/AvailabilitySetData.json similarity index 100% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/AvailabilitySetData.json rename to sdk/core/Azure.Core/perf/TestData/AvailabilitySetData.json diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/ModelXml.xml b/sdk/core/Azure.Core/perf/TestData/ModelXml.xml similarity index 100% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/ModelXml.xml rename to sdk/core/Azure.Core/perf/TestData/ModelXml.xml diff --git a/sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/ResourceProviderData.json b/sdk/core/Azure.Core/perf/TestData/ResourceProviderData.json similarity index 100% rename from sdk/core/Azure.Core/perf/SerializationBenchmark/TestData/ResourceProviderData.json rename to sdk/core/Azure.Core/perf/TestData/ResourceProviderData.json diff --git a/sdk/core/Azure.Core/src/Internal/SequenceWriterAccess.cs b/sdk/core/Azure.Core/src/Internal/SequenceWriterAccess.cs new file mode 100644 index 000000000000..8da747ab8ee9 --- /dev/null +++ b/sdk/core/Azure.Core/src/Internal/SequenceWriterAccess.cs @@ -0,0 +1,106 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.IO; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Azure.Core.Serialization; + +namespace Azure.Core.Internal +{ + internal sealed class SequenceWriterAccess + { + private readonly IModelJsonSerializable _model; + private readonly ModelSerializerOptions _options; + + private SequenceWriter? _writer; + + // write lock is used to synchronize between dispose and serialization + // read lock is used to block disposing while a copy to is in progress + private ReaderWriterLockSlim? _lock; + private ReaderWriterLockSlim Lock => _lock ??= new ReaderWriterLockSlim(); + + public SequenceWriterAccess(IModelJsonSerializable model, ModelSerializerOptions options) + { + _model = model; + _options = options; + } + + private SequenceWriter GetWriter() + { + if (_writer is null) + { + Lock.EnterWriteLock(); + if (_writer is null) + { + SequenceWriter writer = new SequenceWriter(); + using var jsonWriter = new Utf8JsonWriter(writer); + _model.Serialize(jsonWriter, _options); + jsonWriter.Flush(); + _writer = writer; + } + Lock.ExitWriteLock(); + } + return _writer; + } + + public void CopyTo(Stream stream, CancellationToken cancellation) + { + //can't get the write lock from inside the read lock + SequenceWriter writer = GetWriter(); + Lock.EnterReadLock(); + try + { + writer.CopyTo(stream, cancellation); + } + finally + { + Lock.ExitReadLock(); + } + } + + public bool TryComputeLength(out long length) + { + //can't get the write lock from inside the read lock + SequenceWriter writer = GetWriter(); + Lock.EnterReadLock(); + try + { + return writer.TryComputeLength(out length); + } + finally + { + Lock.ExitReadLock(); + } + } + + public async Task CopyToAsync(Stream stream, CancellationToken cancellation) + { + //can't get the write lock from inside the read lock + SequenceWriter writer = GetWriter(); + Lock.EnterReadLock(); + try + { + await writer.CopyToAsync(stream, cancellation).ConfigureAwait(false); + } + finally + { + Lock.ExitReadLock(); + } + } + + public void Dispose() + { + Lock.EnterWriteLock(); + try + { + _writer?.Dispose(); + } + finally + { + Lock.ExitWriteLock(); + } + } + } +} diff --git a/sdk/core/Azure.Core/src/RequestContent.cs b/sdk/core/Azure.Core/src/RequestContent.cs index 35c1dc3083e4..0a0c044a475a 100644 --- a/sdk/core/Azure.Core/src/RequestContent.cs +++ b/sdk/core/Azure.Core/src/RequestContent.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using System.Xml; using Azure.Core.Buffers; +using Azure.Core.Internal; using Azure.Core.Serialization; namespace Azure.Core @@ -101,7 +102,7 @@ public abstract class RequestContent : IDisposable /// The to serialize. /// The to use. /// An instance of that wraps a serialized version of the object. - public static RequestContent Create(IModelJsonSerializable model, ModelSerializerOptions? options = default) => new ModelSerializableContent(model, options ?? ModelSerializerOptions.DefaultWireOptions); + public static RequestContent Create(IModelJsonSerializable model, ModelSerializerOptions? options = default) => new ModelJsonSerializableContent(model, options ?? ModelSerializerOptions.DefaultWireOptions); /// /// Creates an instance of that wraps a serialized version of an object. @@ -237,109 +238,61 @@ public override void Dispose() } } - private sealed class ModelSerializableContent : RequestContent + private sealed class ModelJsonSerializableContent : RequestContent { - private SequenceWriter? _writer; - private BinaryData? _data; - private readonly IModelSerializable _model; + private readonly IModelJsonSerializable _model; private readonly ModelSerializerOptions _options; - private bool _useJsonInterface; - public ModelSerializableContent(IModelSerializable model, ModelSerializerOptions options) - : this(model, options, false) { } - - public ModelSerializableContent(IModelJsonSerializable model, ModelSerializerOptions options) - : this(model, options, true) { } - - private ModelSerializableContent(IModelSerializable model, ModelSerializerOptions options, bool useJsonInterface) + public ModelJsonSerializableContent(IModelJsonSerializable model, ModelSerializerOptions options) { _model = model; _options = options; - _useJsonInterface = useJsonInterface; } - public override void Dispose() => _writer?.Dispose(); + private SequenceWriterAccess? _writerAccess; + private SequenceWriterAccess WriterAccess => _writerAccess ??= new SequenceWriterAccess(_model, _options); - private SequenceWriter GetWriter(IModelJsonSerializable model) - { - if (_writer is null) - { - var writer = new SequenceWriter(); - using var jsonWriter = new Utf8JsonWriter(writer); - model.Serialize(jsonWriter, _options); - jsonWriter.Flush(); - _writer = writer; - } - return _writer; - } + public override void Dispose() => _writerAccess?.Dispose(); + + public override void WriteTo(Stream stream, CancellationToken cancellation) => WriterAccess.CopyTo(stream, cancellation); + + public override async Task WriteToAsync(Stream stream, CancellationToken cancellation) => await WriterAccess.CopyToAsync(stream, cancellation).ConfigureAwait(false); + + public override bool TryComputeLength(out long length) => WriterAccess.TryComputeLength(out length); + } - private BinaryData GetBinaryData() + private sealed class ModelSerializableContent : RequestContent + { + private readonly IModelSerializable _model; + private readonly ModelSerializerOptions _options; + + public ModelSerializableContent(IModelSerializable model, ModelSerializerOptions options) { - if (_data is null) - { - _data = _model.Serialize(_options); - } - return _data; + _model = model; + _options = options; } + public override void Dispose() { } + + private BinaryData? _data; + private BinaryData Data => _data ??= _model.Serialize(_options); + #if NETFRAMEWORK || NETSTANDARD2_0 private byte[]? _bytes; - private byte[] GetBytes() - { - if (_bytes is null) - { - _bytes = GetBinaryData().ToArray(); - } - return _bytes; - } -#endif + private byte[] Bytes => _bytes ??= Data.ToArray(); - public override void WriteTo(Stream stream, CancellationToken cancellation) - { - // a model implements both xml and json we don't know the wire format and must let the model decide. - if (_model is IModelJsonSerializable jsonSerializable && _useJsonInterface) - { - GetWriter(jsonSerializable).CopyTo(stream, cancellation); - } - else - { -#if NETFRAMEWORK || NETSTANDARD2_0 - var bytes = GetBytes(); - stream.Write(bytes, 0, bytes.Length); + public override void WriteTo(Stream stream, CancellationToken cancellation) => stream.Write(Bytes, 0, Bytes.Length); #else - var data = GetBinaryData(); - stream.Write(data.ToMemory().Span); + public override void WriteTo(Stream stream, CancellationToken cancellation) => stream.Write(Data.ToMemory().Span); #endif - } - } public override bool TryComputeLength(out long length) { - if (_model is IModelJsonSerializable jsonSerializable && _useJsonInterface) - { - return GetWriter(jsonSerializable).TryComputeLength(out length); - } - else - { - var data = GetBinaryData(); - - length = data.ToMemory().Length; - return true; - } + length = Data.ToMemory().Length; + return true; } - public override async Task WriteToAsync(Stream stream, CancellationToken cancellation) - { - if (_model is IModelJsonSerializable jsonSerializable && _useJsonInterface) - { - await GetWriter(jsonSerializable).CopyToAsync(stream, cancellation).ConfigureAwait(false); - } - else - { - var data = GetBinaryData(); - await stream.WriteAsync(data.ToMemory(), cancellation).ConfigureAwait(false); - } - } + public override async Task WriteToAsync(Stream stream, CancellationToken cancellation) => await stream.WriteAsync(Data.ToMemory(), cancellation).ConfigureAwait(false); } private sealed class ArrayContent : RequestContent diff --git a/sdk/core/Azure.Core/src/SequenceWriter.cs b/sdk/core/Azure.Core/src/SequenceWriter.cs index cddb55eac010..a6ea05201e61 100644 --- a/sdk/core/Azure.Core/src/SequenceWriter.cs +++ b/sdk/core/Azure.Core/src/SequenceWriter.cs @@ -20,9 +20,9 @@ private struct Buffer public int Written; } - private Buffer[] _buffers; // this is an array so items can be accessed by ref - private int _count; - private int _bufferSize; + private volatile Buffer[] _buffers; // this is an array so items can be accessed by ref + private volatile int _count; + private int _segmentSize; /// /// Initializes a new instance of . @@ -30,7 +30,7 @@ private struct Buffer /// The size of each buffer segment. public SequenceWriter(int segmentSize = 4096) { - _bufferSize = segmentSize; + _segmentSize = segmentSize; _buffers = Array.Empty(); } @@ -62,37 +62,42 @@ public Memory GetMemory(int sizeHint = 0) sizeHint = 256; } - int sizeToRent = sizeHint > _bufferSize ? sizeHint : _bufferSize; + int sizeToRent = sizeHint > _segmentSize ? sizeHint : _segmentSize; if (_buffers.Length == 0) { - _buffers = new Buffer[1]; - _buffers[0].Array = ArrayPool.Shared.Rent(sizeToRent); - _count = 1; + ExpandBuffers(sizeToRent); } ref Buffer last = ref _buffers[_count - 1]; - var free = last.Array.AsMemory(last.Written); + Memory free = last.Array.AsMemory(last.Written); if (free.Length >= sizeHint) { return free; } // else allocate a new buffer: - var newArray = ArrayPool.Shared.Rent(sizeToRent); + ExpandBuffers(sizeToRent); - // add buffer to _buffers - if (_buffers.Length == _count) + return _buffers[_count - 1].Array; + } + + private readonly object _lock = new object(); + private void ExpandBuffers(int sizeToRent) + { + lock (_lock) { - // resize _buffers - var resized = new Buffer[_buffers.Length * 2]; - _buffers.CopyTo(resized, 0); + int bufferCount = _count == 0 ? 1 : _count * 2; + + Buffer[] resized = new Buffer[bufferCount]; + if (_count > 0) + { + _buffers.CopyTo(resized, 0); + } _buffers = resized; + _buffers[_count].Array = ArrayPool.Shared.Rent(sizeToRent); + _count = bufferCount == 1 ? bufferCount : _count + 1; } - - _buffers[_count].Array = newArray; - _count++; - return newArray; } /// @@ -111,15 +116,20 @@ public Span GetSpan(int sizeHint = 0) /// public void Dispose() { - // should we harden it? we really cannot afford use-after-free bugs. they might cause data corruption. - // should we lock other members on this instance when we are disposing? - for (int i = 0; i < _count; i++) + int bufferCountToFree; + Buffer[] buffersToFree; + lock (_lock) { - var buffer = _buffers[i]; - ArrayPool.Shared.Return(buffer.Array); + bufferCountToFree = _count; + buffersToFree = _buffers; + _count = 0; + _buffers = Array.Empty(); + } + + for (int i = 0; i < bufferCountToFree; i++) + { + ArrayPool.Shared.Return(buffersToFree[i].Array); } - _buffers = Array.Empty(); - _count = 0; } /// @@ -128,8 +138,7 @@ public bool TryComputeLength(out long length) length = 0; for (int i = 0; i < _count; i++) { - var buffer = _buffers[i]; - length += buffer.Written; + length += _buffers[i].Written; } return true; } @@ -139,7 +148,7 @@ public void CopyTo(Stream stream, CancellationToken cancellation) { for (int i = 0; i < _count; i++) { - var buffer = _buffers[i]; + Buffer buffer = _buffers[i]; stream.Write(buffer.Array, 0, buffer.Written); } } @@ -149,49 +158,9 @@ public async Task CopyToAsync(Stream stream, CancellationToken cancellation) { for (int i = 0; i < _count; i++) { - var buffer = _buffers[i]; + Buffer buffer = _buffers[i]; await stream.WriteAsync(buffer.Array, 0, buffer.Written).ConfigureAwait(false); } } - - private class MultiBufferSegment : ReadOnlySequenceSegment - { - public MultiBufferSegment(byte[] array, int length, long runningIndex) - { - Memory = new Memory(array, 0, length); - RunningIndex = runningIndex; - } - - public void Add(byte[] array, int length) - { - Next = new MultiBufferSegment(array, length, RunningIndex + Memory.Length); - } - } - - /// - /// Gets a representing the data written to the SequenceWriter. - /// - internal ReadOnlySequence GetReadOnlySequence() - { - if (_count == 0) - { - return ReadOnlySequence.Empty; - } - - if (_count == 1) - { - return new ReadOnlySequence(_buffers[0].Array, 0, _buffers[0].Written); - } - - MultiBufferSegment first = new MultiBufferSegment(_buffers[0].Array, _buffers[0].Written, 0); - MultiBufferSegment previous = first; - for (int i = 1; i < _count; i++) - { - previous!.Add(_buffers[i].Array, _buffers[i].Written); - previous = (MultiBufferSegment)previous.Next!; - } - - return new ReadOnlySequence(first, 0, previous, previous.Memory.Length); - } } } diff --git a/sdk/core/Azure.Core/tests/ModelSerializationContentTests.cs b/sdk/core/Azure.Core/tests/ModelSerializationContentTests.cs index e338a2872e8d..c34fd68ed7b8 100644 --- a/sdk/core/Azure.Core/tests/ModelSerializationContentTests.cs +++ b/sdk/core/Azure.Core/tests/ModelSerializationContentTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Reflection; using Azure.Core.Serialization; using Azure.Core.Tests.Public.ModelSerializationTests.Models; @@ -15,19 +16,17 @@ public class ModelSerializationContentTests [Test] public void CanCalculateLength() { - var modelX = new ModelX("X", "Name", 100, new Dictionary()); + ModelX modelX = new ModelX("X", "Name", 100, new Dictionary()); //use IModelSerializable var content = RequestContent.Create((IModelSerializable)modelX); - bool useJsonContent = (bool)content.GetType().GetField("_useJsonInterface", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(content); - Assert.IsFalse(useJsonContent); + Assert.AreEqual("ModelSerializableContent", content.GetType().Name); content.TryComputeLength(out long lengthNonJson); Assert.Greater(lengthNonJson, 0); //use IModelJsonSerializable var jsonContent = RequestContent.Create((IModelJsonSerializable)modelX); - useJsonContent = (bool)jsonContent.GetType().GetField("_useJsonInterface", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(jsonContent); - Assert.IsTrue(useJsonContent); + Assert.AreEqual("ModelJsonSerializableContent", jsonContent.GetType().Name); content.TryComputeLength(out long lengthJson); Assert.Greater(lengthJson, 0); @@ -35,12 +34,20 @@ public void CanCalculateLength() //use default jsonContent = RequestContent.Create(modelX); - useJsonContent = (bool)jsonContent.GetType().GetField("_useJsonInterface", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(jsonContent); - Assert.IsTrue(useJsonContent); + Assert.AreEqual("ModelJsonSerializableContent", jsonContent.GetType().Name); content.TryComputeLength(out lengthJson); Assert.Greater(lengthJson, 0); Assert.AreEqual(lengthNonJson, lengthJson); } + + [Test] + public void ValidatePrivateClassType() + { + IModelSerializable modelX = new ModelX("X", "Name", 100, new Dictionary()); + + RequestContent content = RequestContent.Create(modelX); + Assert.AreEqual("ModelSerializableContent", content.GetType().Name); + } } } diff --git a/sdk/core/Azure.Core/tests/SequenceWriterTests.cs b/sdk/core/Azure.Core/tests/SequenceWriterTests.cs index 9113b0152a11..c09bdaafca82 100644 --- a/sdk/core/Azure.Core/tests/SequenceWriterTests.cs +++ b/sdk/core/Azure.Core/tests/SequenceWriterTests.cs @@ -2,72 +2,293 @@ // Licensed under the MIT License. using System; -using System.Buffers; +using System.IO; +using System.Reflection; +using System.Threading.Tasks; using NUnit.Framework; namespace Azure.Core.Tests { + /// + /// Uses unique bytes in the memory in each test to better debug any issues with tests not + /// being fully cleaned up and affecting other tests. + /// internal class SequenceWriterTests { [Test] public void ValidateEmptyBuffer() { - SequenceWriter writer = new SequenceWriter(); + using SequenceWriter writer = new SequenceWriter(); Assert.IsTrue(writer.TryComputeLength(out var length)); Assert.AreEqual(0, length); - Assert.AreEqual(ReadOnlySequence.Empty, writer.GetReadOnlySequence()); } [Test] public void ValidateSingleBuffer() { - SequenceWriter writer = new SequenceWriter(512); + using SequenceWriter writer = new SequenceWriter(512); WriteMemory(writer, 400, 0xFF); - var sequence = writer.GetReadOnlySequence(); - - Assert.AreEqual(400, sequence.Length); - Assert.AreEqual(0xFF, sequence.First.Span[0]); - Assert.AreEqual(0xFF, sequence.Slice(399).First.Span[0]); + Assert.IsTrue(writer.TryComputeLength(out var length)); + Assert.AreEqual(400, length); } [Test] public void ValidateMultiBuffer() + { + using SequenceWriter writer = new SequenceWriter(512); + WriteMemory(writer, 400, 0xFE); + WriteMemory(writer, 400, 0xFE); + + Assert.IsTrue(writer.TryComputeLength(out var length)); + Assert.AreEqual(800, length); + } + + [Test] + public void CanWriteMoreThanBufferSize() + { + using SequenceWriter writer = new SequenceWriter(512); + WriteMemory(writer, 513, 0xFD); + WriteMemory(writer, 256, 0xFD); + + Assert.IsTrue(writer.TryComputeLength(out var length)); + Assert.AreEqual(769, length); + } + + [Test] + public void DisposeAfterGetMemory() { SequenceWriter writer = new SequenceWriter(512); - WriteMemory(writer, 400, 0xFF); - WriteMemory(writer, 400, 0xFF); + Memory memory = writer.GetMemory(400); + writer.Dispose(); + + // need to find a way to make this fail + memory.Span.Fill(0xAA); + } + + [Test] + public async Task DisposedWhileCopying() + { + int segments = 10000; + int segmentSize = 400; + SequenceWriter writer = new SequenceWriter(512); + for (int i = 0; i < segments; i++) + { + WriteMemory(writer, segmentSize, 0xFC); + } + + Assert.IsTrue(writer.TryComputeLength(out var length)); + Assert.AreEqual(segments * segmentSize, length); - var sequence = writer.GetReadOnlySequence(); + using MemoryStream memory = new MemoryStream((int)length); + Task result = Task.Run(() => { return writer.CopyToAsync(memory, default); }); - Assert.AreEqual(800, sequence.Length); - Assert.AreEqual(0xFF, sequence.First.Span[0]); - Assert.AreEqual(0xFF, sequence.Slice(399).First.Span[0]); - Assert.AreEqual(0xFF, sequence.Slice(400).First.Span[0]); - Assert.AreEqual(0xFF, sequence.Slice(799).First.Span[0]); + // wait for the writing to start + while (memory.Length == 0) + { } + + writer.Dispose(); + + //Assert.IsTrue(writer.TryComputeLength(out length)); + //Assert.AreEqual(0, length); + + SequenceWriter writer2 = new SequenceWriter(512); + Task result2 = Task.Run(() => + { + for (int i = 0; i < segments; i++) + { + WriteMemory(writer2, segmentSize, 0xFB); + } + }); + + await result; + await result2; + + // because the writer was disposed in the middle of copying to a stream + // the length will be less than expected + // only a portion of the writer was copied to the stream before the dispose + // no exception is thrown here because the writer is not thread safe + Assert.Greater(memory.Length, 0); + Assert.LessOrEqual(memory.Length, length); // in rare cases it can be equal + byte[] memoryStreamBuffer = memory.GetBuffer(); + for (int i = 0; i < memory.Length; i++) + { + // sometimes fails due to rerent, dispose doesn't wait for copies to finish + // failed on 102nd try + Assert.AreEqual(0xFC, memoryStreamBuffer[i]); + } + writer2.Dispose(); } [Test] - public void CanWriteMoreThanBufferSize() + public async Task DisposedWhileWriting() { + int segments = 10000; + int segmentSize = 400; SequenceWriter writer = new SequenceWriter(512); - WriteMemory(writer, 513, 0xFF); - WriteMemory(writer, 256, 0xFE); + bool exceptionThrown = false; + Task result = Task.Run(() => + { + try + { + for (int i = 0; i < segments; i++) + { + WriteMemory(writer, segmentSize, 0xFA); + } + } + catch (IndexOutOfRangeException) + { + // due to not fully locking and making the writer thread safe + // it is possible to get an index out of range exception + // if you dispose the writer right after a resize + exceptionThrown = true; + } + }); + long sequenceLength = 0; + do + { + Assert.IsTrue(writer.TryComputeLength(out sequenceLength)); + } while (sequenceLength == 0); + + writer.Dispose(); + + // The original task will continue to write to the sequence writer + // but the writer was disposed in the middle so whatever was written before + // the dispose will be freed back to the shared pool so the length will be less than expected + await result; + + Assert.IsTrue(writer.TryComputeLength(out sequenceLength)); + if (!exceptionThrown) + { + Assert.Greater(sequenceLength, 0); + Assert.Less(sequenceLength, segments * segmentSize); + } + } + + [Test] + public void GetNewMemoryAfterDispose() + { + SequenceWriter writer = new SequenceWriter(512); + writer.Dispose(); Assert.IsTrue(writer.TryComputeLength(out var length)); - Assert.AreEqual(769, length); + Assert.AreEqual(0, length); - var sequence = writer.GetReadOnlySequence(); - Assert.AreEqual(769, sequence.Length); - Assert.AreEqual(0xFF, sequence.First.Span[0]); - Assert.AreEqual(0xFE, sequence.Slice(514).First.Span[0]); + // should this fail with ObjectDisposedException? + WriteMemory(writer, 400, 0xF9); + Assert.IsTrue(writer.TryComputeLength(out length)); + Assert.AreEqual(400, length); + + writer.Dispose(); } private static void WriteMemory(SequenceWriter writer, int size, byte data) { - var memory = writer.GetMemory(size); + Memory memory = writer.GetMemory(size); memory.Span.Slice(0, size).Fill(data); writer.Advance(size); } + + [Test] + public void GetMultipleMemoryWithoutAdvance() + { + using SequenceWriter writer = new SequenceWriter(512); + Memory memory1 = writer.GetMemory(400); + + // should we throw here on the second GetMemory without calling Advance? + // It seems that you should not be able to get more memory without advancing + // If we did this perhaps we could eliminate the use-after-free and worst case leak + // one segment that doesn't get disposed + Memory memory2 = writer.GetMemory(400); + memory2.Span.Fill(0xF8); + writer.Advance(400); + + MemoryStream stream = new MemoryStream(400); + writer.CopyTo(stream, default); + Assert.AreEqual(400, stream.Length); + byte[] buffer = stream.GetBuffer(); + + // even though we write to the second memory buffer the first segment had 0 written bytes + Assert.AreEqual(0xF8, buffer[0]); + Assert.AreEqual(0xF8, buffer[399]); + + Array rawSegments = writer.GetType().GetField("_buffers", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(writer) as Array; + // we should have 1 segment since the two memory objects should be pointed at the same place + Assert.AreEqual(1, rawSegments.Length); + + memory1.Span.Fill(0xF7); + + stream = new MemoryStream(400); + writer.CopyTo(stream, default); + Assert.AreEqual(400, stream.Length); + buffer = stream.GetBuffer(); + + // without advancing since the first and second Memory pointed to the same place + // we have replaced what was there + Assert.AreEqual(0xF7, buffer[0]); + Assert.AreEqual(0xF7, buffer[399]); + + Assert.Throws(() => writer.Advance(400)); + } + + [Test] + public void GetMultipleMemoryMultipleSegmentWithoutAdvance() + { + using SequenceWriter writer = new SequenceWriter(512); + Memory memory1 = writer.GetMemory(400); + memory1.Span.Fill(0); //zero it out for validation later + + // should we throw here on the second GetMemory without calling Advance? + // It seems that you should not be able to get more memory without advancing + // If we did this perhaps we could eliminate the use-after-free and worst case leak + // one segment that doesn't get disposed + Memory memory2 = writer.GetMemory(800); + memory2.Span.Fill(0xF6); + writer.Advance(800); + + MemoryStream stream = new MemoryStream(800); + writer.CopyTo(stream, default); + Assert.AreEqual(800, stream.Length); + byte[] buffer = stream.GetBuffer(); + + // even though we write to the second memory buffer the first segment had 0 written bytes + Assert.AreEqual(0xF6, buffer[0]); + Assert.AreEqual(0xF6, buffer[799]); + + Array rawSegments = writer.GetType().GetField("_buffers", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(writer) as Array; + // we should have 2 segment since the second memory was larger than the segment size + Assert.AreEqual(2, rawSegments.Length); + object buffer1 = rawSegments.GetValue(0); + int buffer1BytesWritten = (int)buffer1.GetType().GetField("Written", BindingFlags.Public | BindingFlags.Instance).GetValue(buffer1); + Assert.AreEqual(0, buffer1BytesWritten); + + byte[] buffer1Array = (byte[])buffer1.GetType().GetField("Array", BindingFlags.Public | BindingFlags.Instance).GetValue(buffer1); + Assert.AreEqual(0, buffer1Array[0]); + Assert.AreEqual(0, buffer1Array[399]); + + object buffer2 = rawSegments.GetValue(1); + int buffer2BytesWritten = (int)buffer2.GetType().GetField("Written", BindingFlags.Public | BindingFlags.Instance).GetValue(buffer2); + Assert.AreEqual(800, buffer2BytesWritten); + + byte[] buffer2Array = (byte[])buffer2.GetType().GetField("Array", BindingFlags.Public | BindingFlags.Instance).GetValue(buffer2); + + memory1.Span.Fill(0xF5); + + // Since we didn't advance the public view is still the same + // but internally we should have stuff written to the first segment + stream = new MemoryStream(800); + writer.CopyTo(stream, default); + Assert.AreEqual(800, stream.Length); + buffer = stream.GetBuffer(); + Assert.AreEqual(0xF6, buffer[0]); + Assert.AreEqual(0xF6, buffer[799]); + + buffer1Array = (byte[])buffer1.GetType().GetField("Array", BindingFlags.Public | BindingFlags.Instance).GetValue(buffer1); + Assert.AreEqual(0xF5, buffer1Array[0]); + Assert.AreEqual(0xF5, buffer1Array[399]); + + if (buffer2Array.Length < 1200) //sometimes the buffer will be big enough + Assert.Throws(() => writer.Advance(400)); + } } }