-
Notifications
You must be signed in to change notification settings - Fork 899
Filters stream rebound #948
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.
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
Changes from all commits
703a8f7
08f2ff4
d2da645
e0177f5
849a417
fbfc5d1
3e8bb70
7336dab
91c1a79
1a3e4c9
a803a1f
5e02299
3b84e2d
616ab06
1238c81
1adcff1
89d9908
8eb623b
bf51ca9
2b2c0f8
10df3b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,4 +93,4 @@ private static Blob CommitOnBranchAndReturnDatabaseBlob(Repository repo, string | |
return blob; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,53 +1,91 @@ | ||
using System; | ||
using System.IO; | ||
using System.Runtime.InteropServices; | ||
using LibGit2Sharp.Core.Handles; | ||
|
||
namespace LibGit2Sharp.Core | ||
{ | ||
/// <summary> | ||
/// Writes data to a <see cref="GitBuf"/> pointer | ||
/// </summary> | ||
internal class GitBufWriteStream : MemoryStream | ||
internal class GitBufWriteStream : Stream | ||
{ | ||
private readonly IntPtr gitBufPointer; | ||
|
||
internal GitBufWriteStream(IntPtr gitBufPointer) | ||
{ | ||
this.gitBufPointer = gitBufPointer; | ||
|
||
//Preallocate the buffer | ||
Proxy.git_buf_grow(gitBufPointer, 1024); | ||
} | ||
|
||
public override void Flush() | ||
{ | ||
} | ||
|
||
protected override void Dispose(bool disposing) | ||
public override long Seek(long offset, SeekOrigin origin) | ||
{ | ||
if (base.CanSeek) // False if stream has already been written/closed | ||
throw new NotSupportedException(); | ||
} | ||
|
||
public override void SetLength(long value) | ||
{ | ||
throw new NotSupportedException(); | ||
} | ||
|
||
public override int Read(byte[] buffer, int offset, int count) | ||
{ | ||
throw new NotSupportedException(); | ||
} | ||
|
||
public override void Write(byte[] buffer, int offset, int count) | ||
{ | ||
AutoGrowBuffer(count); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Allocating a page would be better yes (assuming you do mean 4kB). The difference in allocations is surprising, since you're also growing by 1.5 and the size is ensured anyhow. The one difference seems to be that you assume there is going to be a next request and preallocate for it. How large is the difference in reallocs? What's the difference in memory consumption? |
||
|
||
Proxy.git_buf_put(gitBufPointer, buffer, offset, count); | ||
} | ||
|
||
private void AutoGrowBuffer(int count) | ||
{ | ||
var gitBuf = gitBufPointer.MarshalAs<GitBuf>(); | ||
|
||
var asize = (uint)gitBuf.asize; | ||
var size = (uint)gitBuf.size; | ||
|
||
var isBufferLargeEnoughToHoldTheNewData = (asize - size) > count; | ||
var filledBufferPercentage = (100.0 * size / asize); | ||
|
||
if (isBufferLargeEnoughToHoldTheNewData && filledBufferPercentage < 90) | ||
{ | ||
using (var gitBuf = gitBufPointer.MarshalAs<GitBuf>()) | ||
{ | ||
WriteTo(gitBuf); | ||
} | ||
return; | ||
} | ||
|
||
base.Dispose(disposing); | ||
var targetSize = (uint)(1.5 * (asize + count)); | ||
|
||
Proxy.git_buf_grow(gitBufPointer, targetSize); | ||
} | ||
|
||
private void WriteTo(GitBuf gitBuf) | ||
public override bool CanRead | ||
{ | ||
Seek(0, SeekOrigin.Begin); | ||
get { return false; } | ||
} | ||
|
||
var length = (int)Length; | ||
var bytes = new byte[length]; | ||
Read(bytes, 0, length); | ||
public override bool CanSeek | ||
{ | ||
get { return false; } | ||
} | ||
|
||
IntPtr reverseBytesPointer = Marshal.AllocHGlobal(length); | ||
Marshal.Copy(bytes, 0, reverseBytesPointer, bytes.Length); | ||
public override bool CanWrite | ||
{ | ||
get { return true; } | ||
} | ||
|
||
var size = (UIntPtr)length; | ||
var allocatedSize = (UIntPtr)length; | ||
NativeMethods.git_buf_set(gitBuf, reverseBytesPointer, size); | ||
gitBuf.size = size; | ||
gitBuf.asize = allocatedSize; | ||
public override long Length | ||
{ | ||
get { throw new NotSupportedException(); } | ||
} | ||
|
||
Marshal.StructureToPtr(gitBuf, gitBufPointer, true); | ||
public override long Position | ||
{ | ||
get { throw new NotSupportedException(); } | ||
set { throw new NotSupportedException(); } | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
e0902fbce7d14631bd02091c1c70cde3e68f78ab | ||
a2012c43899e6616366b47d7741b3f035e825e84 |
+4 −0 | CHANGELOG.md | |
+2 −0 | CMakeLists.txt | |
+3 −1 | README.md | |
+8 −9 | appveyor.yml | |
+4 −2 | examples/describe.c | |
+3 −0 | examples/for-each-ref.c | |
+10 −0 | include/git2/buffer.h | |
+32 −0 | include/git2/config.h | |
+3 −3 | include/git2/diff.h | |
+3 −6 | include/git2/patch.h | |
+2 −2 | include/git2/repository.h | |
+4 −1 | include/git2/sys/repository.h | |
+0 −97 | src/bswap.h | |
+0 −1 | src/buffer.h | |
+1 −0 | src/checkout.c | |
+0 −1 | src/common.h | |
+41 −0 | src/config.c | |
+2 −1 | src/diff_patch.c | |
+1 −1 | src/hash/hash_generic.c | |
+3 −0 | src/index.c | |
+3 −0 | src/openssl_stream.c | |
+6 −0 | src/pathspec.c | |
+1 −1 | src/pool.c | |
+7 −5 | src/repository.c | |
+3 −0 | tests/checkout/nasty.c | |
+5 −0 | tests/clar.c | |
+22 −0 | tests/clar_libgit2.c | |
+4 −0 | tests/clar_libgit2.h | |
+46 −0 | tests/config/read.c | |
+36 −6 | tests/core/structinit.c | |
+3 −0 | tests/path/win32.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the
GitBuf
is owned by libgit2, so we shouldn't touch it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. See #946 (comment)