Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Faster byte[] copy #511

Closed
wants to merge 1 commit into from
Closed

Faster byte[] copy #511

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
3 changes: 2 additions & 1 deletion src/Microsoft.AspNet.Server.Kestrel/Filter/SocketInputStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using IllyriadGames.ByteArrayExtensions;
Copy link
Member

Choose a reason for hiding this comment

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

??? 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VectorizedCopy is in a standalone lib as it has a wide applicable use in many different situations; adding a reference seemed easier than including the source here, or Common and trying to workout what do with the file headers (here and in the standalone lib). Happy to take advice 😀

Hopefully the clr will change so it isn't needed; and everything can get the benefit, but equally that might either be a change to memmove in the C Standard Lib; or will be coreclr only for stability, so I'm not holding my breath on timelines 😦

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added reference as specific version number; so the code referenced is under nuget's control not mine 😉

Though would probably be "safer" to use the source; but... dotnet/corefx#3199 (diff) don't know what the situation is.

Copy link
Member

Choose a reason for hiding this comment

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

But you know we're not going to have a dependency on "IllyriadGames.ByteArrayExtensions": "1.0.0-beta-10" in our web server right? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doubt it 😄

What about a source file with a different copyright notice; but using same licence (Apache License, Version 2.0)? Not sure you'd be comfortable with the precedent either... Everyone would want a header! 😝

But only adding the Copyright (c) .NET Foundation. header would mean I'd be in the perverse situation of sub-licensing VectorizedCopy and would need to retain the header if I was to use it under Apache 2.0? (as I understand it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want to drop net451 and move to only supporting net46? I've just been made aware of Buffer.MemoryCopy which should offer better performance... (Doesn't appear till 4.6 and coreclr)

Could ifdef it I suppose...

Copy link
Contributor

Choose a reason for hiding this comment

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

net46 will likely break Mono. Not necessarily the most recent released version or master, but quite likely the one release that comes most closely to working: 4.0.x.

Copy link
Member

Choose a reason for hiding this comment

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

We're going to be moving to .NET 4.6 for RC2. We'll need to see how much of mono breaks when we do that. Hopefully 4.2.x will have enough 4.6 support to run.

Copy link
Contributor

Choose a reason for hiding this comment

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

(sorry for continuing here)
Switching to .NET 4.6 will break current Mono users. The switch to the .NET 4.6 reference source only happened back in October, so all versions of Mono (including the most recent 4.2 and the mostly-working 4.0) don't have the Array.Empty member. Things will go back the way they were in beta2: aspnet/dnx#1105

Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is known. I'm working with the mono guys to figure out which version of mono will support .NET 4.6 reference assemblies. Hopefully by the time RC2 is ready we'll have a version of mono that supports that.

using Microsoft.AspNet.Server.Kestrel.Http;
using Microsoft.AspNet.Server.Kestrel.Infrastructure;

Expand Down Expand Up @@ -76,7 +77,7 @@ public override void Write(byte[] buffer, int offset, int count)
{
var inputBuffer = _socketInput.IncomingStart(count);

Buffer.BlockCopy(buffer, offset, inputBuffer.Data.Array, inputBuffer.Data.Offset, count);
buffer.VectorizedCopy(offset, inputBuffer.Data.Array, inputBuffer.Data.Offset, count);

_socketInput.IncomingComplete(count, error: null);
}
Expand Down
9 changes: 5 additions & 4 deletions src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolIterator2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Diagnostics;
using System.Numerics;
using IllyriadGames.ByteArrayExtensions;

namespace Microsoft.AspNet.Server.Kestrel.Infrastructure
{
Expand Down Expand Up @@ -552,7 +553,7 @@ public MemoryPoolIterator2 CopyTo(byte[] array, int offset, int count, out int a
actual = count;
if (array != null)
{
Buffer.BlockCopy(block.Array, index, array, offset, remaining);
block.Array.VectorizedCopy(index, array, offset, remaining);
}
return new MemoryPoolIterator2(block, index + remaining);
}
Expand All @@ -561,15 +562,15 @@ public MemoryPoolIterator2 CopyTo(byte[] array, int offset, int count, out int a
actual = count - remaining + following;
if (array != null)
{
Buffer.BlockCopy(block.Array, index, array, offset, following);
block.Array.VectorizedCopy(index, array, offset, following);
}
return new MemoryPoolIterator2(block, index + following);
}
else
{
if (array != null)
{
Buffer.BlockCopy(block.Array, index, array, offset, following);
block.Array.VectorizedCopy(index, array, offset, following);
}
offset += following;
remaining -= following;
Expand Down Expand Up @@ -619,7 +620,7 @@ public void CopyFrom(byte[] data, int offset, int count)

var bytesToCopy = remaining < bytesLeftInBlock ? remaining : bytesLeftInBlock;

Buffer.BlockCopy(data, bufferIndex, block.Array, blockIndex, bytesToCopy);
data.VectorizedCopy(bufferIndex, block.Array, blockIndex, bytesToCopy);

blockIndex += bytesToCopy;
bufferIndex += bytesToCopy;
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.AspNet.Server.Kestrel/project.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"Microsoft.AspNet.Internal.libuv-Windows": {
"version": "1.0.0-*",
"type": "build"
}
},
"IllyriadGames.ByteArrayExtensions": "1.0.0-beta-10"
},
"frameworks": {
"dnx451": { },
Expand Down