Merge pull request #626 from qmfrederik/experiment/packfile-no-memory…

…-mapping GitPack: Don't use memory mapped streams when accessing pack files
dotnet · Jul 27, 2021 · f90c41c · f90c41c
2 parents 41696ba + c955e1f
commit f90c41c
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 16 deletions.
diff --git a/src/NerdBank.GitVersioning.Tests/ManagedGit/GitPackTests.cs b/src/NerdBank.GitVersioning.Tests/ManagedGit/GitPackTests.cs
@@ -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)));
@@ -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)));
@@ -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)));
+                }
             }
         }
 

diff --git a/src/NerdBank.GitVersioning/ManagedGit/GitPack.cs b/src/NerdBank.GitVersioning/ManagedGit/GitPack.cs
@@ -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>();
@@ -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>
@@ -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);
         }
@@ -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();
         }
 
@@ -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()