How a fix in Go 1.9 sped up our Gitaly service by 30x (opens in new tab)

I suspect you are misunderstanding "exec" as "shell". China router firmwares call the shell.

Putting arbitrary input into a shell is dangerous, as missed escaping can result in control of the shell.

When you call exec yourself, however, you are passing the individual arguments as NULL-terminated list of strings (char*). There is no shell to abuse. Calling a process this way is about as safe as calling a function that takes strings for arguments. The function can still have vulnerabilities, but the process of calling it is safe.

Do they just do that for commands that make changes, or do they do it for pure read commands as well? Most of the volume is in reads, especially since so many build systems now read directly from Github.

Spawning a process, even if its just vfork() followed by exec(), is very very expensive compared to spawning a thread, and even then most apps created threads on startup and then just cache them for the lifetime of the application. Spawning a process 20 times a second honestly isn't terrible, but even hundreds per second can be noticeably slow IIRC.

Process isolation is good for security.

Parsing text data in ad-hoc, non-standardized, not documented, not defined format is really bad for security.

Just spawning a process creates as many security problems as it solves.

If it was done right, it would look like Chrome architecture, where untrusted, isolated processes can do dangerous work but communicate with trusted process via well defined IPC protocol.

> It's beneficial for security

... and for RAM usage. Java applications all have a tendency to bloat the longer you keep them running.

I'm on the Gitaly team.

As zegerjan wrote, Gitaly is a Go/Ruby hybrid.

The main Go process doesn't use libgit2 (for now) because we didn't want to have to deal with cgo. We already know how to deal with C extensions in Ruby, and we have a lot of existing Ruby application code that uses libgit2, so we still use it there. And that code works fine so I don't see us removing it.

In practice, sometimes spawning a Git process is faster than using libgit2, so why then not do that. Also for parts of our workload (handling Git push/pull operations), spawning a one-off process (git-upload-pack) is the most boring / tried-and-true approach.

I'm at GitLab but not on the Gitaly team. I think we are using libgit2 but that it doesn't contain all the calls we need.

The Gitaly server has a Ruby component, but also a Go component. The Ruby server uses Rugged[1] and Gollum-lib[2] which both use libgit2.

The Go component doesn't have libgit2 binding yet, although we're looking into adding that later. That or maybe go-git[3]. But for now Gitaly is mainly focussed at migrating all git calls from the Rails monolith. Not introducing a new component now reduces the risks this project has.

[1]: https://github.com/libgit2/rugged/ [2]: https://gitlab.slack.com/archives/C027N716H/p151695430400026... [3]: https://github.com/src-d/go-git/

Assuming they're currently pure git, binding to libgit2 would require using cgo, which could have far-ranging (negative) consequences.

libgit2 has had bugs related to file locking (for example when you make some ref changes while garbage collecting) and libgit2 does not implement all the git features, so you cannot do everything using libgit2 anyway.

This is a terrible microbenchmark.

First of all, you're only benchmarking the time it takes for fork(2) to return in the parent subshell, nothing else. The new processes don't exist yet at this point, and certainly hasn't exec'd (which tends to be why you're forking).

Second, you're not measuring the cost at all. The forked children will, at some point, start executing on other CPUs, which includes finishing configuration and running exec, which takes time. The cost is the total cycles it takes before the child is executing the intended code.

Fork is damn expensive, but whether they're too expensive depends on the usecase, and the cost of expanding hardware.

Fork time scales with the virtual memory of the forking process, and you're forking from a fresh subshell that hardly has anything allocated. It's even mentioned in the linked post that their issue stemmed from this (specifically fork lock contention spiking as fork time increased).

I must say I didn't go to look at the sources of the patch, but what you say sounds so odd that I'll take the chance and suggest that perhaps the fact that in golang "amd64" is, for historical reasons, the name of the architecture more neutrally known as "x86_64", is the source of confusion (I.e. it doesn't just work on AMD or on CPUs that claim/report having a specific model/maker etc).

Low level syscall ABI is architecture dependent.

Go uses system calls directly because the alternative in UNIX land is linking with libc.

fork was alright before other people tacked on multiples of cruft like threads and whatnot onto commercial unixes and they became mainstream. the current problem is that you don't want to have to copy all file descriptors if all you're going to do is call "exec" and reduce them to three: in, out, err.

for example, here's the caveats section from the macOS fork man page:

     There are limits to what you can do in the child process.  To be totally safe you should restrict your-
     self to only executing async-signal safe operations until such time as one of the exec functions is
     called.  All APIs, including global data symbols, in any framework or library should be assumed to be
     unsafe after a fork() unless explicitly documented to be safe or async-signal safe.  If you need to use
     these frameworks in the child process, you must exec.  In this situation it is reasonable to exec your-
     self.

That spells defeat :)

Earlier in the game, copy-on-write had to be created for the same reasons.

Another nice example is changing the working directory for the new process. With fork+exec, you can do a chdir after fork but before exec. With posix_spawn you're stuck with the working directory of the parent.

Nowadays, posix_spawn() calls clone() on linux, with the CLONE_VM flag, behaving much like vfork() as far as I can tell.

That means the child and parent process shares the memory (until exec() is performed).

Especially if the parent process is multi-threaded this avoids a whole lot of pagefaults that would occur if using fork() when another thread touches memory, possibly triggering a lot of copy-on-writes in the time window between calling fork() and the child calling exec()

Code: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/uni...

They wouldn't need to stop the world and do a full rewrite. It would be feasible if they stop writing new components in Ruby and began replacing the existing parts piecemeal.