Bring back some pointers

[carlosmn]

This serves as a PoC for how we could be using pointers to access the libgit2 data instead of marshaling structures into managed land and using SafeHandles which are meant for system handles, not the usual CRT-allocated structures.

This starts with a few small things, but we can keep bringing this in bit by bit.

/cc @nulltoken @whoisj

[whoisj]

Senior Carlos, you're on fire this morning.

This looks great.

I have sometime similar cooking as a pure proof of concept (not likely to go anywhere) to help me test how thin a layer can be created between a project like LG2 and .NET managed software.

[nulltoken]

@carlosmn Neat job! However, I'm not seeing a net reduction in the number of lines of code. Although this is quite easy to read for something at ease with C, I'm not sure this would be that easy for a .NET contributor. What's the real benefit of this? Do we get a neat perf bump? Something else? In other words, what would make the conversion to unsafe worth the porting effort?

[carlosmn]

There won't be a big reduction in lines of code even after we remove the redundant Git... classes, but that's not the primary motivator. The code which performs all the copies is hidden behind MarshalAs<T>() and the special handling of SafeHandle by the runtime; if you count those calls as the number of copies it performs, we would count that as a significant reduction.

The goals are twofold:

1. Get rid of the SafeHandle usage. These are meant for OS resources and are handled especially by the runtime. We should not be using them for libgit2 objects, which are all malloc'ed memory (with the possible exception of writable streams, but even then the resource is held by libgit2 not by us).
2. Get rid of copying/marshaling to managed memory just to copy again. We use SafeHandle or IntPtr and MarshalAs<T>() to bring in "unsafe" memory into "safe" managed memory. But then we're just copying the data again from the Git... objects into the public structs. Making all that copying useless, using up CPU IO time and causing managed allocations we don't need. We should get a perf bump from avoiding these copies, though with everything else going on it might be hard to measure definitively.

Getting rid of the SafeHandles means we have to either move to pointers in the type system or do everything with IntPtr. While the latter might look more familiar for a "normal" C# developer, the end result is the same. We are dealing with pointers, and doing the wrong operation is going to segfault regardless of whether the method has to be marked unsafe or not.

Regardless of whether we spell something git_index_entry* or IntPtr, the actual knowledge of how unmanaged memory works which you need to have to do anything at the interop layer is the same. And with the actual pointer type we let the type system help us figure out what can be dereferenced, rather than having the user perform managed-looking casts.

If you look at the code for retrieving the output of git_remote_ls(), I'd argue that the "unsafe" code is easier to read as we use an array the way the C code expected you to, and can do

for (int i = 0; i < intCount; i++)
{
    git_remote_head* currentHead = heads[i];
}

instead of

for (int i = 0; i < intCount; i++)
{
    currentHead = IntPtr.Add(currentHead, IntPtr.Size);
}

What we're doing in the "safe" code is manually stepping through the array by manually adding memory addresses instead of the accessing it like a list.

Maybe not everything will be immediately better with "unsafe" pointers, but let's not kid ourselves that coding the interop layer is actually safer when we have untyped pointers. There's a reason we don't just have opaque types be void* in libgit2 (type checking in the compiler). But that is what we're forgoing if we use IntPtr everywhere.

[Therzok]

Style: Use PascalCase for function names.

[whoisj]

I'm not sure this would be that easy for a .NET contributor

I'm not sure that should be our first concern. The majority of contributions should be in the layer above the proxy. Ideally the proxy would be a very thin shim between the managed world of C# and native world of Libgit2, and (nearly) nothing more.

If that is the case, C# developers should have no problems adding new features to the pure C# layer at the top of the library's stack.

[ethomson]

Manually merged...!