Make the rot13 test filter stream its output #958
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nulltoken
changed the title
|
/cc @ammeep |
This will flush out chunks of 4kB to the underlying writer
|
Applying the following patch on top of this PR diff --git a/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs b/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
index f2fc14b..d21b1e8 100644
--- a/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
+++ b/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
@@ -11,8 +11,31 @@ public class FilterSubstitutionCipherFixture : BaseFixture
[Fact]
public void CorrectlyEncodesAndDecodesInput()
{
- const string decodedInput = "This is a substitution cipher";
- const string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
+ string decodedInput = "This is a substitution cipher";
+ string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
+
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
var attributes = new List<string> { ".rot13" };
var filter = new SubstitutionCipherFilter("ROT13", attributes);
@@ -25,7 +48,9 @@ public void CorrectlyEncodesAndDecodesInput()
using (var repo = new Repository(repoPath, repositoryOptions))
{
+ Console.WriteLine("Before Stage() call");
var blob = CommitOnBranchAndReturnDatabaseBlob(repo, fileName, decodedInput);
+ Console.WriteLine("After Stage() call");
var textDetected = blob.GetContentText();
Assert.Equal(encodedInput, textDetected);
@@ -37,7 +62,9 @@ public void CorrectlyEncodesAndDecodesInput()
DeleteFile(repo, fileName);
+ Console.WriteLine("Before Checkout() call");
repo.Checkout("master");
+ Console.WriteLine("After Checkout() call");
var fileContents = ReadTextFromFile(repo, fileName);
Assert.Equal(1, filter.SmudgeCalledCount);
diff --git a/LibGit2Sharp/Core/GitBufWriteStream.cs b/LibGit2Sharp/Core/GitBufWriteStream.cs
index f727fec..e31f743 100644
--- a/LibGit2Sharp/Core/GitBufWriteStream.cs
+++ b/LibGit2Sharp/Core/GitBufWriteStream.cs
@@ -37,6 +37,9 @@ public override int Read(byte[] buffer, int offset, int count)
public override void Write(byte[] buffer, int offset, int count)
{
+ Console.WriteLine("-------");
+ Console.WriteLine("Count={0}", count);
+
AutoGrowBuffer(count);
Proxy.git_buf_put(gitBufPointer, buffer, offset, count);
@@ -60,6 +63,10 @@ private void AutoGrowBuffer(int count)
var targetSize = (ulong)(1.5 * (asize + ulongCount));
+ Console.WriteLine("Size={0}", size);
+ Console.WriteLine("ASize={0}", asize);
+ Console.WriteLine("TargetSize={0}", targetSize);
+
Proxy.git_buf_grow(gitBufPointer, targetSize);
}
Will output the following when running the rot13 test |
|
Since you're using 90% of the buffer being filled after adding to it, the last auto-grow you perform in C# is unnecessary, and you waste about 80kB altogether. It's not too big a deal, as the OS often just won't actually give you that memory, but it could be an issue in 32-bit mode if you handle a lot of large files, as you might waste several MB of address space. If you skipped the auto-grow and let the |
I agree, but that would lead to realloc the native buffer with each output of 4KB. With current implementation, for one I'm not a memory wizard. Fewer reallocs or fewer wasted memory... Which one would be better? I'll gladly update the PR accordingly. |
How do you figure that? |
|
@carlosmn Dammit! You're right. I forgot that Dropped the managed implementation. Applying the following patch diff --git a/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs b/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
index f2fc14b..d21b1e8 100644
--- a/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
+++ b/LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
@@ -11,8 +11,31 @@ public class FilterSubstitutionCipherFixture : BaseFixture
[Fact]
public void CorrectlyEncodesAndDecodesInput()
{
- const string decodedInput = "This is a substitution cipher";
- const string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
+ string decodedInput = "This is a substitution cipher";
+ string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
+
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
+ decodedInput += decodedInput;
+ encodedInput += encodedInput;
var attributes = new List<string> { ".rot13" };
var filter = new SubstitutionCipherFilter("ROT13", attributes);
@@ -25,7 +48,9 @@ public void CorrectlyEncodesAndDecodesInput()
using (var repo = new Repository(repoPath, repositoryOptions))
{
+ Console.WriteLine("Before Stage() call");
var blob = CommitOnBranchAndReturnDatabaseBlob(repo, fileName, decodedInput);
+ Console.WriteLine("After Stage() call");
var textDetected = blob.GetContentText();
Assert.Equal(encodedInput, textDetected);
@@ -37,7 +62,9 @@ public void CorrectlyEncodesAndDecodesInput()
DeleteFile(repo, fileName);
+ Console.WriteLine("Before Checkout() call");
repo.Checkout("master");
+ Console.WriteLine("After Checkout() call");
var fileContents = ReadTextFromFile(repo, fileName);
Assert.Equal(1, filter.SmudgeCalledCount);
diff --git a/LibGit2Sharp/Core/GitBufWriteStream.cs b/LibGit2Sharp/Core/GitBufWriteStream.cs
index 7cc281f..f4c7204 100644
--- a/LibGit2Sharp/Core/GitBufWriteStream.cs
+++ b/LibGit2Sharp/Core/GitBufWriteStream.cs
@@ -37,6 +37,17 @@ public override int Read(byte[] buffer, int offset, int count)
public override void Write(byte[] buffer, int offset, int count)
{
+ var gitBuf = gitBufPointer.MarshalAs<GitBuf>();
+
+ var ulongCount = Convert.ToUInt64(count);
+ var asize = (ulong)gitBuf.asize;
+ var size = (ulong)gitBuf.size;
+
+ Console.WriteLine("-------");
+ Console.WriteLine("Count={0}", count);
+ Console.WriteLine("Size={0}", size);
+ Console.WriteLine("Asize={0}", asize);
+
Proxy.git_buf_put(gitBufPointer, buffer, offset, count);
}now ouputs the following |
|
Down to 6 reallocs (with much less wasted memory) when staging. |
|
This is fantastic ✨ I'm happy to merge this in if you are done with it? |
|
I'm done. Merge at your will! |
|
💖 Thanks you |
ammeep
pushed a commit
that referenced
this pull request
Feb 16, 2015
Make the rot13 test filter stream its output
|
Wow... You're fast! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Various enhancements to the test filters