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

C#: add System.Memory dependency #5835

Merged
merged 1 commit into from
Mar 18, 2019
Merged

C#: add System.Memory dependency #5835

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
33 changes: 22 additions & 11 deletions csharp/src/Google.Protobuf/ByteString.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,6 @@ internal static ByteString FromBytes(byte[] bytes)
{
return new ByteString(bytes);
}

/// <summary>
/// Provides direct, unrestricted access to the bytes contained in this instance.
/// You must not modify or resize the byte array returned by this method.
/// </summary>
internal static byte[] GetBuffer(ByteString bytes)
{
return bytes.bytes;
}
}

/// <summary>
Expand Down Expand Up @@ -119,6 +110,14 @@ public bool IsEmpty
get { return Length == 0; }
}

#if NETSTANDARD2_0
/// <summary>
/// Provides read-only access to the data of this <see cref="ByteString"/>.
/// No data is copied so this is the most efficient way of accessing.
/// </summary>
public ReadOnlySpan<byte> Span => new ReadOnlySpan<byte>(bytes);
#endif

/// <summary>
/// Converts this <see cref="ByteString"/> into a byte array.
/// </summary>
Expand Down Expand Up @@ -161,7 +160,7 @@ public static ByteString FromStream(Stream stream)
int capacity = stream.CanSeek ? checked((int) (stream.Length - stream.Position)) : 0;
var memoryStream = new MemoryStream(capacity);
stream.CopyTo(memoryStream);
#if NETSTANDARD1_0
#if NETSTANDARD1_0 || NETSTANDARD2_0
byte[] bytes = memoryStream.ToArray();
#else
// Avoid an extra copy if we can.
Expand All @@ -187,7 +186,7 @@ public static ByteString FromStream(Stream stream)
// 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_0
#if NETSTANDARD1_0 || NETSTANDARD2_0
byte[] bytes = memoryStream.ToArray();
#else
// Avoid an extra copy if we can.
Expand Down Expand Up @@ -219,6 +218,18 @@ public static ByteString CopyFrom(byte[] bytes, int offset, int count)
return new ByteString(portion);
}

#if NETSTANDARD2_0
/// <summary>
/// Constructs a <see cref="ByteString" /> from a read only span. The contents
/// are copied, so further modifications to the span will not
/// be reflected in the returned <see cref="ByteString" />.
/// </summary>
public static ByteString CopyFrom(ReadOnlySpan<byte> bytes)
{
return new ByteString(bytes.ToArray());
}
#endif

/// <summary>
/// Creates a new <see cref="ByteString" /> by encoding the specified text with
/// the given encoding.
Expand Down
8 changes: 6 additions & 2 deletions csharp/src/Google.Protobuf/Google.Protobuf.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<VersionPrefix>3.7.0</VersionPrefix>
<LangVersion>6</LangVersion>
<Authors>Google Inc.</Authors>
<TargetFrameworks>netstandard1.0;net45</TargetFrameworks>
<TargetFrameworks>netstandard1.0;netstandard2.0;net45</TargetFrameworks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<AssemblyOriginatorKeyFile>../../keys/Google.Protobuf.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
Expand All @@ -28,9 +28,13 @@
- Visual Studio.
-->
<PropertyGroup Condition="'$(OS)' != 'Windows_NT'">
<TargetFrameworks>netstandard1.0</TargetFrameworks>
<TargetFrameworks>netstandard1.0;netstandard2.0</TargetFrameworks>
</PropertyGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
<PackageReference Include="System.Memory" Version="4.5.2" />
Copy link

Choose a reason for hiding this comment

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

Adding a new dependency is also usually considered a "breaking change". Maybe make this conditional on the target framework?

Copy link

Choose a reason for hiding this comment

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

Adding a new dependency is also usually considered a "breaking change". Maybe make this conditional on the target framework?

Adding a new dependency isn't a breaking change. Removing one is though. I think we should potentially bump the minor version here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor version will be bumped because this change won't be available before protobuf v3.8.x is out.

</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" PrivateAssets="All" />
</ItemGroup>
Expand Down