diff --git a/Makefile.am b/Makefile.am index 4fc706bc9c..4e0452af4c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -89,6 +89,7 @@ csharp_EXTRA_DIST= \ csharp/src/Google.Protobuf.Benchmarks/BenchmarkDatasetConfig.cs \ csharp/src/Google.Protobuf.Benchmarks/BenchmarkMessage1Proto3.cs \ csharp/src/Google.Protobuf.Benchmarks/Benchmarks.cs \ + csharp/src/Google.Protobuf.Benchmarks/ByteStringBenchmark.cs \ csharp/src/Google.Protobuf.Benchmarks/Google.Protobuf.Benchmarks.csproj \ csharp/src/Google.Protobuf.Benchmarks/GoogleMessageBenchmark.cs \ csharp/src/Google.Protobuf.Benchmarks/ParseMessagesBenchmark.cs \ @@ -171,6 +172,7 @@ csharp_EXTRA_DIST= \ csharp/src/Google.Protobuf.sln \ csharp/src/Google.Protobuf/ByteArray.cs \ csharp/src/Google.Protobuf/ByteString.cs \ + csharp/src/Google.Protobuf/ByteStringAsync.cs \ csharp/src/Google.Protobuf/CodedInputStream.cs \ csharp/src/Google.Protobuf/CodedOutputStream.ComputeSize.cs \ csharp/src/Google.Protobuf/CodedOutputStream.cs \ @@ -268,7 +270,8 @@ csharp_EXTRA_DIST= \ csharp/src/Google.Protobuf/WriteContext.cs \ csharp/src/Google.Protobuf/WriteBufferHelper.cs \ csharp/src/Google.Protobuf/UnknownField.cs \ - csharp/src/Google.Protobuf/UnknownFieldSet.cs + csharp/src/Google.Protobuf/UnknownFieldSet.cs \ + csharp/src/Google.Protobuf/UnsafeByteOperations.cs java_EXTRA_DIST= \ java/README.md \ diff --git a/csharp/src/Google.Protobuf.Benchmarks/ByteStringBenchmark.cs b/csharp/src/Google.Protobuf.Benchmarks/ByteStringBenchmark.cs new file mode 100644 index 0000000000..a755850a66 --- /dev/null +++ b/csharp/src/Google.Protobuf.Benchmarks/ByteStringBenchmark.cs @@ -0,0 +1,72 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2019 Google Inc. All rights reserved. +// https://github.com/protocolbuffers/protobuf +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +using BenchmarkDotNet.Attributes; + +namespace Google.Protobuf.Benchmarks +{ + /// + /// Benchmarks using ByteString. + /// + [MemoryDiagnoser] + public class ByteStringBenchmark + { + private const int Zero = 0; + private const int Kilobyte = 1024; + private const int _128Kilobytes = 1024 * 128; + private const int Megabyte = 1024 * 1024; + private const int _10Megabytes = 1024 * 1024 * 10; + + byte[] byteBuffer; + + [GlobalSetup] + public void GlobalSetup() + { + byteBuffer = new byte[PayloadSize]; + } + + [Params(Zero, Kilobyte, _128Kilobytes, Megabyte, _10Megabytes)] + public int PayloadSize { get; set; } + + [Benchmark] + public ByteString CopyFrom() + { + return ByteString.CopyFrom(byteBuffer); + } + + [Benchmark] + public ByteString UnsafeWrap() + { + return UnsafeByteOperations.UnsafeWrap(byteBuffer); + } + } +} diff --git a/csharp/src/Google.Protobuf.Test/ByteStringTest.cs b/csharp/src/Google.Protobuf.Test/ByteStringTest.cs index adbbc3ef54..bed8c8b8aa 100644 --- a/csharp/src/Google.Protobuf.Test/ByteStringTest.cs +++ b/csharp/src/Google.Protobuf.Test/ByteStringTest.cs @@ -34,6 +34,9 @@ using System; using System.Text; using NUnit.Framework; using System.IO; +using System.Collections.Generic; +using System.Collections; +using System.Linq; #if !NET35 using System.Threading.Tasks; #endif @@ -54,6 +57,7 @@ namespace Google.Protobuf EqualityTester.AssertInequality(b1, b3); EqualityTester.AssertInequality(b1, b4); EqualityTester.AssertInequality(b1, null); + EqualityTester.AssertEquality(ByteString.Empty, ByteString.Empty); #pragma warning disable 1718 // Deliberately calling ==(b1, b1) and !=(b1, b1) Assert.IsTrue(b1 == b1); Assert.IsTrue(b1 == b2); @@ -63,6 +67,7 @@ namespace Google.Protobuf Assert.IsTrue((ByteString) null == null); Assert.IsFalse(b1 != b1); Assert.IsFalse(b1 != b2); + Assert.IsTrue(ByteString.Empty == ByteString.Empty); #pragma warning disable 1718 Assert.IsTrue(b1 != b3); Assert.IsTrue(b1 != b4); @@ -154,6 +159,65 @@ namespace Google.Protobuf Assert.AreEqual(3, bs[1]); } + [Test] + public void CopyTo() + { + byte[] data = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; + ByteString bs = ByteString.CopyFrom(data); + + byte[] dest = new byte[data.Length]; + bs.CopyTo(dest, 0); + + CollectionAssert.AreEqual(data, dest); + } + + [Test] + public void GetEnumerator() + { + byte[] data = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; + ByteString bs = ByteString.CopyFrom(data); + + IEnumerator genericEnumerator = bs.GetEnumerator(); + Assert.IsTrue(genericEnumerator.MoveNext()); + Assert.AreEqual(0, genericEnumerator.Current); + + IEnumerator enumerator = ((IEnumerable)bs).GetEnumerator(); + Assert.IsTrue(enumerator.MoveNext()); + Assert.AreEqual(0, enumerator.Current); + + // Call via LINQ + CollectionAssert.AreEqual(bs.Span.ToArray(), bs.ToArray()); + } + + [Test] + public void UnsafeWrap() + { + byte[] data = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; + ByteString bs = UnsafeByteOperations.UnsafeWrap(data.AsMemory(2, 3)); + ReadOnlySpan s = bs.Span; + + Assert.AreEqual(3, s.Length); + Assert.AreEqual(2, s[0]); + Assert.AreEqual(3, s[1]); + Assert.AreEqual(4, s[2]); + + // Check that the value is not a copy + data[2] = byte.MaxValue; + Assert.AreEqual(byte.MaxValue, s[0]); + } + + [Test] + public void WriteToStream() + { + byte[] data = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; + ByteString bs = ByteString.CopyFrom(data); + + MemoryStream ms = new MemoryStream(); + bs.WriteTo(ms); + + CollectionAssert.AreEqual(data, ms.ToArray()); + } + [Test] public void ToStringUtf8() { diff --git a/csharp/src/Google.Protobuf/ByteString.cs b/csharp/src/Google.Protobuf/ByteString.cs index 45b3885a5c..2619be5bef 100644 --- a/csharp/src/Google.Protobuf/ByteString.cs +++ b/csharp/src/Google.Protobuf/ByteString.cs @@ -34,6 +34,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.IO; +using System.Runtime.InteropServices; using System.Security; using System.Text; #if !NET35 @@ -49,40 +50,26 @@ namespace Google.Protobuf /// /// Immutable array of bytes. /// + [SecuritySafeCritical] public sealed class ByteString : IEnumerable, IEquatable { private static readonly ByteString empty = new ByteString(new byte[0]); - private readonly byte[] bytes; + private readonly ReadOnlyMemory bytes; /// - /// Unsafe operations that can cause IO Failure and/or other catastrophic side-effects. + /// Internal use only. Ensure that the provided memory is not mutated and belongs to this instance. /// - internal static class Unsafe - { - /// - /// Constructs a new ByteString from the given byte array. The array is - /// *not* copied, and must not be modified after this constructor is called. - /// - internal static ByteString FromBytes(byte[] bytes) - { - return new ByteString(bytes); - } - } - - /// - /// Internal use only. Ensure that the provided array is not mutated and belongs to this instance. - /// - internal static ByteString AttachBytes(byte[] bytes) + internal static ByteString AttachBytes(ReadOnlyMemory bytes) { return new ByteString(bytes); } /// - /// Constructs a new ByteString from the given byte array. The array is + /// Constructs a new ByteString from the given memory. The memory is /// *not* copied, and must not be modified after this constructor is called. /// - private ByteString(byte[] bytes) + private ByteString(ReadOnlyMemory bytes) { this.bytes = bytes; } @@ -117,11 +104,7 @@ namespace Google.Protobuf /// public ReadOnlySpan Span { - [SecuritySafeCritical] - get - { - return new ReadOnlySpan(bytes); - } + get { return bytes.Span; } } /// @@ -130,11 +113,7 @@ namespace Google.Protobuf /// public ReadOnlyMemory Memory { - [SecuritySafeCritical] - get - { - return new ReadOnlyMemory(bytes); - } + get { return bytes; } } /// @@ -144,7 +123,7 @@ namespace Google.Protobuf /// A byte array with the same data as this ByteString. public byte[] ToByteArray() { - return (byte[]) bytes.Clone(); + return bytes.ToArray(); } /// @@ -153,7 +132,16 @@ namespace Google.Protobuf /// A base64 representation of this ByteString. public string ToBase64() { - return Convert.ToBase64String(bytes); + if (MemoryMarshal.TryGetArray(bytes, out ArraySegment segment)) + { + // Fast path. ByteString was created with an array, so pass the underlying array. + return Convert.ToBase64String(segment.Array, segment.Offset, segment.Count); + } + else + { + // Slow path. BytesString is not an array. Convert memory and pass result to ToBase64String. + return Convert.ToBase64String(bytes.ToArray()); + } } /// @@ -197,21 +185,10 @@ namespace Google.Protobuf /// The stream to copy into a ByteString. /// The cancellation token to use when reading from the stream, if any. /// A ByteString with content read from the given stream. - public async static Task FromStreamAsync(Stream stream, CancellationToken cancellationToken = default(CancellationToken)) + public static Task FromStreamAsync(Stream stream, CancellationToken cancellationToken = default(CancellationToken)) { ProtoPreconditions.CheckNotNull(stream, nameof(stream)); - int capacity = stream.CanSeek ? checked((int) (stream.Length - stream.Position)) : 0; - var memoryStream = new MemoryStream(capacity); - // We have to specify the buffer size here, as there's no overload accepting the cancellation token - // alone. But it's documented to use 81920 by default if not specified. - await stream.CopyToAsync(memoryStream, 81920, cancellationToken); -#if NETSTANDARD1_1 || NETSTANDARD2_0 - byte[] bytes = memoryStream.ToArray(); -#else - // Avoid an extra copy if we can. - byte[] bytes = memoryStream.Length == memoryStream.Capacity ? memoryStream.GetBuffer() : memoryStream.ToArray(); -#endif - return AttachBytes(bytes); + return ByteStringAsync.FromStreamAsyncCore(stream, cancellationToken); } #endif @@ -242,7 +219,6 @@ namespace Google.Protobuf /// are copied, so further modifications to the span will not /// be reflected in the returned . /// - [SecuritySafeCritical] public static ByteString CopyFrom(ReadOnlySpan bytes) { return new ByteString(bytes.ToArray()); @@ -270,7 +246,7 @@ namespace Google.Protobuf /// public byte this[int index] { - get { return bytes[index]; } + get { return bytes.Span[index]; } } /// @@ -284,7 +260,18 @@ namespace Google.Protobuf /// The result of decoding the binary data with the given decoding. public string ToString(Encoding encoding) { - return encoding.GetString(bytes, 0, bytes.Length); + if (MemoryMarshal.TryGetArray(bytes, out ArraySegment segment)) + { + // Fast path. ByteString was created with an array. + return encoding.GetString(segment.Array, segment.Offset, segment.Count); + } + else + { + // Slow path. BytesString is not an array. Convert memory and pass result to GetString. + // TODO: Consider using GetString overload that takes a pointer. + byte[] array = bytes.ToArray(); + return encoding.GetString(array, 0, array.Length); + } } /// @@ -304,9 +291,10 @@ namespace Google.Protobuf /// Returns an iterator over the bytes in this . /// /// An iterator over the bytes in this object. + [SecuritySafeCritical] public IEnumerator GetEnumerator() { - return ((IEnumerable) bytes).GetEnumerator(); + return MemoryMarshal.ToEnumerable(bytes).GetEnumerator(); } /// @@ -324,7 +312,17 @@ namespace Google.Protobuf public CodedInputStream CreateCodedInput() { // We trust CodedInputStream not to reveal the provided byte array or modify it - return new CodedInputStream(bytes); + if (MemoryMarshal.TryGetArray(bytes, out ArraySegment segment) && segment.Count == bytes.Length) + { + // Fast path. ByteString was created with a complete array. + return new CodedInputStream(segment.Array); + } + else + { + // Slow path. BytesString is not an array, or is a slice of an array. + // Convert memory and pass result to WriteRawBytes. + return new CodedInputStream(bytes.ToArray()); + } } /// @@ -343,18 +341,8 @@ namespace Google.Protobuf { return false; } - if (lhs.bytes.Length != rhs.bytes.Length) - { - return false; - } - for (int i = 0; i < lhs.Length; i++) - { - if (rhs.bytes[i] != lhs.bytes[i]) - { - return false; - } - } - return true; + + return lhs.bytes.Span.SequenceEqual(rhs.bytes.Span); } /// @@ -373,6 +361,7 @@ namespace Google.Protobuf /// /// The object to compare this with. /// true if refers to an equal ; false otherwise. + [SecuritySafeCritical] public override bool Equals(object obj) { return this == (obj as ByteString); @@ -383,12 +372,15 @@ namespace Google.Protobuf /// will return the same hash code. /// /// A hash code for this object. + [SecuritySafeCritical] public override int GetHashCode() { + ReadOnlySpan b = bytes.Span; + int ret = 23; - foreach (byte b in bytes) + for (int i = 0; i < b.Length; i++) { - ret = (ret * 31) + b; + ret = (ret * 31) + b[i]; } return ret; } @@ -403,20 +395,12 @@ namespace Google.Protobuf return this == other; } - /// - /// Used internally by CodedOutputStream to avoid creating a copy for the write - /// - internal void WriteRawBytesTo(CodedOutputStream outputStream) - { - outputStream.WriteRawBytes(bytes, 0, bytes.Length); - } - /// /// Copies the entire byte array to the destination array provided at the offset specified. /// public void CopyTo(byte[] array, int position) { - ByteArray.Copy(bytes, 0, array, position, bytes.Length); + bytes.CopyTo(array.AsMemory(position)); } /// @@ -424,7 +408,17 @@ namespace Google.Protobuf /// public void WriteTo(Stream outputStream) { - outputStream.Write(bytes, 0, bytes.Length); + if (MemoryMarshal.TryGetArray(bytes, out ArraySegment segment)) + { + // Fast path. ByteString was created with an array, so pass the underlying array. + outputStream.Write(segment.Array, segment.Offset, segment.Count); + } + else + { + // Slow path. BytesString is not an array. Convert memory and pass result to WriteRawBytes. + var array = bytes.ToArray(); + outputStream.Write(array, 0, array.Length); + } } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/ByteStringAsync.cs b/csharp/src/Google.Protobuf/ByteStringAsync.cs new file mode 100644 index 0000000000..8bf8add4b8 --- /dev/null +++ b/csharp/src/Google.Protobuf/ByteStringAsync.cs @@ -0,0 +1,64 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Google.Protobuf +{ + /// + /// SecuritySafeCritical attribute can not be placed on types with async methods. + /// This class has ByteString's async methods so it can be marked with SecuritySafeCritical. + /// + internal static class ByteStringAsync + { +#if !NET35 + internal static async Task FromStreamAsyncCore(Stream stream, CancellationToken cancellationToken) + { + int capacity = stream.CanSeek ? checked((int)(stream.Length - stream.Position)) : 0; + var memoryStream = new MemoryStream(capacity); + // We have to specify the buffer size here, as there's no overload accepting the cancellation token + // alone. But it's documented to use 81920 by default if not specified. + await stream.CopyToAsync(memoryStream, 81920, cancellationToken); +#if NETSTANDARD1_1 || NETSTANDARD2_0 + byte[] bytes = memoryStream.ToArray(); +#else + // Avoid an extra copy if we can. + byte[] bytes = memoryStream.Length == memoryStream.Capacity ? memoryStream.GetBuffer() : memoryStream.ToArray(); +#endif + return ByteString.AttachBytes(bytes); + } +#endif + } +} \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/UnsafeByteOperations.cs b/csharp/src/Google.Protobuf/UnsafeByteOperations.cs new file mode 100644 index 0000000000..5ef25d4a18 --- /dev/null +++ b/csharp/src/Google.Protobuf/UnsafeByteOperations.cs @@ -0,0 +1,81 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion + +using System; +using System.Security; + +namespace Google.Protobuf +{ + /// + /// Provides a number of unsafe byte operations to be used by advanced applications with high performance + /// requirements. These methods are referred to as "unsafe" due to the fact that they potentially expose + /// the backing buffer of a to the application. + /// + /// + /// + /// The methods in this class should only be called if it is guaranteed that the buffer backing the + /// will never change! Mutation of a can lead to unexpected + /// and undesirable consequences in your application, and will likely be difficult to debug. Proceed with caution! + /// + /// + /// This can have a number of significant side affects that have spooky-action-at-a-distance-like behavior. In + /// particular, if the bytes value changes out from under a Protocol Buffer: + /// + /// + /// + /// serialization may throw + /// + /// + /// serialization may succeed but the wrong bytes may be written out + /// + /// + /// messages are no longer threadsafe + /// + /// + /// hashCode may be incorrect + /// + /// + /// + [SecuritySafeCritical] + public static class UnsafeByteOperations + { + /// + /// Constructs a new from the given bytes. The bytes are not copied, + /// and must not be modified while the is in use. + /// This API is experimental and subject to change. + /// + public static ByteString UnsafeWrap(ReadOnlyMemory bytes) + { + return ByteString.AttachBytes(bytes); + } + } +}