So, now that I’m finished my optimizations, it’s time to send them upstream. How does this work? Different projects have different methods of submission, but one of the most common (and the one that’s relevant for my situation) is the pull request on github. Basically, you fork the project (creating a copy of it on your own account), make some changes, and then a pull request asks upstream to merge those changes into their repository. In practice, you can generally expect one of four things to happen when you make a pull request:
- They fully approve of your changes, and hit the ‘merge’ button without any further discussion. Ideal, though unlikely to happen unless you’ve discussed it ahead of time (or are already familiar with submitting code to the project).
- They don’t approve it immediately. Instead, they request that you make changes before the pull request is accepted. These changes can be anything from small tweaks (like coding style changes, or other simple things) to more fundamental reworks that will take more effort. I’d say this is probably the most common result – expect that your pull request won’t be perfect on the first try.
- They don’t like your changes at all and close the pull request without merging it. Assuming your pull request is reasonably thought out (and that you’ve tested it properly), this doesn’t usually happen.
- There’s no reply and the pull request simply sits open on github. For slower projects this isn’t too surprising. If the project isn’t completely dead, this will likely be replaced by one of the first three options eventually. People maintaining open source get busy too, especially if it’s a project with a single (or few) developers.
As a side note, I would have communicated more with the csnappy developers before this point, but they don’t seem to have an IRC channel or any other contact information, so I’m not sure where the project is discussed. In retrospect I could have opened an issue on github, though issues are typically used more as bug reports for the developer to solve, so I didn’t think of it at the time.
Anyway, here are some tips for a successful pull request:
- Make one commit per change. Use a descriptive commit name. For example, I used ‘Use 64bit load/store for UnalignedCopy64 on AArch64’ as my second commit name. Reading it gives you a pretty good idea what’s going on. You can also elaborate a bit in the description field – in this case, I put ‘ This results in a slight performance gain.’ Doing this makes it easy for other people to know what’s going on and what you’ve changed.
- If your project has guidelines for submissions, look them up and follow them. There aren’t any that I can find for csnappy, so I don’t have much more to say here (plus, I already did a couple blog posts about this near the beginning of this blog). Another thing worth looking for is coding style guidelines – following those can save you time, and save the people running the project time.
- Have a glance through previous pull requests on the project to get an idea what to expect. This can be useful, especially if the project does not have posted submission guidelines.
- Provide detail, but don’t provide irrelevant detail. My pull request is simply titled ‘Minor AArch64 Optimizations’, with the text ‘From my tests, these changes increase compression speed by roughly 1.3%.’ You don’t need to explain everything about the process, just what you’ve done and why + what the results are. If this was a more in depth or potentially controversial pull request, I would spend some more time explaining what I’ve done and why it’s worth it, but for something this simple, a one line description (plus the commits themselves) should suffice.
- If you’re contributing multiple things, split them up into multiple pull requests. This makes it easier to test and handle your pull requests. For example, if I was performing significant optimizations and code changes to both the compression and the decompression algorithms, it would make sense to do separate pull requests. Though, if both were simply adding AArch64 to existing preprocessor defines, I’d likely keep them as a single pull request.
Anyway, I’m running out of advice to give, so without any further ado, here’s the pull request I ended up with: https://github.com/zeevt/csnappy/pull/34
I imagine a pull request as simple as this will just be accepted when someone with access sees it, but we’ll see. I’ll make another blog post whenever the csnappy maintainers reply to or accept the pull request.