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

Jump to bottom

GitPack: Don't use memory mapped streams when accessing pack files #626

Merged
merged 2 commits into from Jul 27, 2021
Merged

GitPack: Don't use memory mapped streams when accessing pack files #626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edc1621
GitPack: Don't use MemoryMappedFiles in 32-bit processes
qmfrederik Jul 23, 2021
c955e1f
Make sure Streams are disposed of.
qmfrederik Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 34 additions & 8 deletions src/NerdBank.GitVersioning.Tests/ManagedGit/GitPackTests.cs
View file
Open in desktop
Expand Up @@ -63,7 +63,15 @@ public void GetPackedObject()
// This commit is not deltafied. It is stored as a .gz-compressed stream in the pack file.
var zlibStream = Assert.IsType<ZLibStream>(commitStream);
var deflateStream = Assert.IsType<DeflateStream>(zlibStream.BaseStream);
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);

if (IntPtr.Size > 4)
{
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);
}
else
{
var pooledStream = Assert.IsType<FileStream>(deflateStream.BaseStream);
}

Assert.Equal(222, commitStream.Length);
Assert.Equal("/zgldANj+jvgOwlecnOKylZDVQg=", Convert.ToBase64String(sha.ComputeHash(commitStream)));
Expand All @@ -85,7 +93,15 @@ public void GetDeltafiedObject()
var deltaStream = Assert.IsType<GitPackDeltafiedStream>(commitStream);
var zlibStream = Assert.IsType<ZLibStream>(deltaStream.BaseStream);
var deflateStream = Assert.IsType<DeflateStream>(zlibStream.BaseStream);
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);

if (IntPtr.Size > 4)
{
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);
}
else
{
var directAccessStream = Assert.IsType<FileStream>(deflateStream.BaseStream);
}

Assert.Equal(137, commitStream.Length);
Assert.Equal("lZu/7nGb0n1UuO9SlPluFnSvj4o=", Convert.ToBase64String(sha.ComputeHash(commitStream)));
Expand Down Expand Up @@ -120,14 +136,24 @@ public void TryGetObjectTest()
using (SHA1 sha = SHA1.Create())
{
Assert.True(gitPack.TryGetObject(GitObjectId.Parse("f5b401f40ad83f13030e946c9ea22cb54cb853cd"), "commit", out Stream commitStream));
using (commitStream)
{
// This commit is not deltafied. It is stored as a .gz-compressed stream in the pack file.
var zlibStream = Assert.IsType<ZLibStream>(commitStream);
var deflateStream = Assert.IsType<DeflateStream>(zlibStream.BaseStream);

// This commit is not deltafied. It is stored as a .gz-compressed stream in the pack file.
var zlibStream = Assert.IsType<ZLibStream>(commitStream);
var deflateStream = Assert.IsType<DeflateStream>(zlibStream.BaseStream);
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);
if (IntPtr.Size > 4)
{
var pooledStream = Assert.IsType<MemoryMappedStream>(deflateStream.BaseStream);
}
else
{
var directAccessStream = Assert.IsType<FileStream>(deflateStream.BaseStream);
}

Assert.Equal(222, commitStream.Length);
Assert.Equal("/zgldANj+jvgOwlecnOKylZDVQg=", Convert.ToBase64String(sha.ComputeHash(commitStream)));
Assert.Equal(222, commitStream.Length);
Assert.Equal("/zgldANj+jvgOwlecnOKylZDVQg=", Convert.ToBase64String(sha.ComputeHash(commitStream)));
}
}
}

Expand Down
39 changes: 31 additions & 8 deletions src/NerdBank.GitVersioning/ManagedGit/GitPack.cs
View file
Open in desktop
Expand Up @@ -31,8 +31,8 @@ public class GitPack : IDisposable
private readonly Func<FileStream> packStream;
private readonly Lazy<FileStream> indexStream;
private readonly GitPackCache cache;
private MemoryMappedFile packFile;
private MemoryMappedViewAccessor accessor;
private MemoryMappedFile? packFile = null;
private MemoryMappedViewAccessor? accessor = null;

// Maps GitObjectIds to offets in the git pack.
private readonly Dictionary<GitObjectId, long> offsets = new Dictionary<GitObjectId, long>();
Expand Down Expand Up @@ -98,8 +98,11 @@ public GitPack(GetObjectFromRepositoryDelegate getObjectFromRepositoryDelegate,
this.indexStream = indexStream ?? throw new ArgumentNullException(nameof(indexStream));
this.cache = cache ?? new GitPackMemoryCache();

this.packFile = MemoryMappedFile.CreateFromFile(this.packStream(), mapName: null, 0, MemoryMappedFileAccess.Read, HandleInheritability.None, leaveOpen: false);
this.accessor = this.packFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.Read);
if (IntPtr.Size > 4)
{
this.packFile = MemoryMappedFile.CreateFromFile(this.packStream(), mapName: null, 0, MemoryMappedFileAccess.Read, HandleInheritability.None, leaveOpen: false);
this.accessor = this.packFile.CreateViewAccessor(0, 0, MemoryMappedFileAccess.Read);
}
}

/// <summary>
Expand Down Expand Up @@ -202,7 +205,17 @@ public Stream GetObject(long offset, string objectType)
}

var packStream = this.GetPackStream();
Stream objectStream = GitPackReader.GetObject(this, packStream, offset, objectType, packObjectType);
Stream objectStream;

try
{
objectStream = GitPackReader.GetObject(this, packStream, offset, objectType, packObjectType);
}
catch
{
packStream.Dispose();
throw;
}

return this.cache.Add(offset, objectStream);
}
Expand Down Expand Up @@ -240,8 +253,8 @@ public void Dispose()
this.indexReader.Value.Dispose();
}

this.accessor.Dispose();
this.packFile.Dispose();
this.accessor?.Dispose();
this.packFile?.Dispose();
this.cache.Dispose();
}

Expand All @@ -265,7 +278,17 @@ public void Dispose()

private Stream GetPackStream()
{
return new MemoryMappedStream(this.accessor);
// On 64-bit processes, we can use Memory Mapped Streams (the address space
// will be large enough to map the entire packfile). On 32-bit processes,
// we directly access the underlying stream.
if (IntPtr.Size > 4)
{
return new MemoryMappedStream(this.accessor);
}
else
{
return this.packStream();
}
}

private GitPackIndexReader OpenIndex()
Expand Down