Welcome to the 27th edition of Git Rev News, a digest of all things Git. For our goals, the archives, the way we work, and how to contribute or to subscribe, see the Git Rev News page on git.github.io.
This edition covers what happened during the month of April 2017.
At the end of last January, René Scharfe sent a 5 patch long patch series to the mailing list. The goal of this series was to make a swap macro available to the entire code base, and to use it to simplify existing code.
As “exchanging the value of two variables requires declaring a temporary variable and repeating their names”, the new macro would swap the value of two variables, but “reduce repetition and hide the declaration of temporary variables”.
The actual implementation that René wanted to make globally available was the following one:
#define SWAP(a, b) do { \
void *_swap_a_ptr = &(a); \
void *_swap_b_ptr = &(b); \
unsigned char _swap_buffer[sizeof(a)]; \
memcpy(_swap_buffer, _swap_a_ptr, sizeof(a)); \
memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) + \
BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \
memcpy(_swap_b_ptr, _swap_buffer, sizeof(a)); \
} while (0)
It looks complex and maybe slower than using a temporary variable of the same type as the type of the two variables that we want to swap, but René wrote that it “makes sure that the storage sizes of its two parameters are the same” while “its memcpy() calls are optimized away by current compilers”.
To that last statement Johannes Schindelin, alias Dscho, who is the Git for Windows maintainer, replied:
How certain are we that “current compilers” optimize that away? And about which “current compilers” are we talking? GCC? GCC 6? Clang? Clang 3? Clang 3.8.*? Visual C 2005+?
René pointed Dscho to a web site showing the assembler generated by gcc and clang which shows that on x86-64 they “compile the macro to the same object code as a manual swap”, and guessed that as Dscho had introduced the original macro using memcpy() in the first place, it should be optimized by Microsoft compilers.
In the meantime Dscho also sent a simpler looking implementation saying:
It may seem as a matter of taste, or maybe not: I prefer this without the _swap_a_ptr (and I would also prefer not to use identifiers starting with an underscore, as section 7.1.3 Reserved Identifiers of the C99 standard says they are reserved)
To the above René replied:
We can move the underscore to the end, but using a and b directly will give surprising results if the parameters have side effects.
Johannes Sixt, alias Hannes, agreed on that with René saying that his implementation “avoid that the macro arguments are evaluated more than once”.
To Hannes, Dscho replied that as the macro arguments are passed to sizeof(), he mistakenly thought that arguments with side effects would be rejected by compilers:
It is curious, though, that an expression like “sizeof(a++)” would not be rejected.
Further, what would SWAP(a++, b) do? Swap a and b, and then increment a?
René answered that gcc and clang warn or error out in that last case, and Dscho further commented that anyway “it does not make much sense to support SWAP(a++, b), as it is confusing at best”.
Brandon Williams then chimed in to suggest placing a comment at the definition of the macro to “disallow a side-effect operator in the macro”.
In response René further noticed that while “SWAP(a++, …) is caught by the compiler, SWAP(*a++, …) works fine”.
The above in turn prompted Jeff King, alias Peff, to suggest having the macro “take pointers-to-objects, making it look more like a real function (i.e., SWAP(&a, &b) instead of SWAP(a, b))”. But Junio Hamano and Dscho were not sure that it would improve things.
Meanwhile Dscho agreed that his commit, which introduced such a macro in the first place, was not a good idea:
The commit you quoted embarrasses me, and I have no excuse for it. I would love to see that myswap() ugliness fixed by replacing it with a construct that is simpler, and generates good code even without any smart compiler.
René benchmarked a test program using the macro and found that indeed when not enabling all the optimizations there could be some performance regression.
Dscho then suggested a macro with a type parameter:
#define SIMPLE_SWAP(T, a, b) do { T tmp_ = a; a = b; b = tmp_; } while (0)
stating:
It would be trivially “optimized” out of the box, even when compiling with Tiny C or in debug mode.
The discussion further continued in February on related topics like:
but René’s SWAP macro got merged.
In April though the discussion restarted about what should happen with “SWAP(foo[i], foo[j])” when i == j, and when swapping overlapping memory.
These later discussions involved only René, Duy Nguyen and Peff who sent a patch to avoid swapping an element with itself, and another one to avoid calling memcpy on overlapping objects in the macro.
Carlos Pita sent an email to the Git mailing list starting with:
I’m currently using ‘git annex’ to manage my entire file collection (including tons of music and books) and I noticed how slow autocompletion has become for files in the index (say for git add).
Then Carlos presented some changes to the __git_index_files()
function in “git-completion.bash” that appear to speed up this
function from 0.83 seconds to 0.35 seconds in his repository.
In a latter email he suggested a further change that reduce the time further to only 0.08 seconds, that is 10 times faster than the original time.
The changes Carlos showed use sed instead of “a while-read-case-echo
bash loop” though, and he wondered if the Git developers wanted to
avoid depending on sed.
Ævar Arnfjörð Bjarmason replied:
It’s fine to depend on sed, these shell-scripts are POSIX compatible, and so is sed, we use sed in a lot of the built-in shell scripts.
and then encouraged Carlos to submit a patch with his improvements.
Jacob Keller, alias Jake, seconded Ævar’s opinion. Junio Hamano, the
Git maintainer, agreed, too – “as long as the use of sed is in line
with POSIX.1”. Junio then suggested using sed’s -e option instead of
its -r to improve portability.
Johannes Sixt, alias Hannes, though replied to Carlos that:
This is about command line completion. We go a long way to avoid forking processes there. What is 10x faster on Linux despite of forking a process may not be so on Windows.
Jake then suggested using different implementations on Windows and Linux to get the best speed on each platform, while Junio replied that the speed of the different implementation also depends on how many paths have to be processed which depends on the repository size:
If there are only a few paths, the loop in shell would beat a pipe into sed even on Linux, I suspect, and if there are tons of paths, at some number, loop in shell would become slower than a single spawning of sed on platforms with slower fork, no?
Hannes agreed with Junio saying:
Absolutely. I just want to make sure a suggested change takes into account the situation on Windows, not only the “YESSSS!” and “VERY WELL!” votes of Linux users ;)
The discussion stopped at that point, but hopefully it will lead to some auto completion improvements soon.
I’m a hobbyist programmer living in Bochum, Germany. I learned programming in C mostly by reading Linux kernel diffs and trying to keep up with its mailing list. Not long after Linus announced Git there in 2005 I started contributing patches. In my day job as a DBA I started to actually use Git only this year.
Probably git-tar-tree, which later became git archive, had the highest impact. It allows users to easily package their work for non-Git-users. And as my first patch it got me hooked!
I’m also quite fond of the -W/–function-context options of git diff and git grep, even though they still get comments wrong. Meaningful context (as opposed to a fixed number of lines) makes the most sense to me, but I don’t know how widely these options are used when there are IDEs, websites like GitHub and tools like cscope.
There is a modest pile of cleanup patches fermenting on my PC that I’d like to get rid of. Sometimes I get into a cleanup frenzy, but not all of the patches make sense on the next morning and some conflict with “real” changes in flight, so usually I don’t send them right away.
I’m creating semantic patches for some of them to let Coccinelle do the actual cleanup work, in the hope that the addressed issues are then basically fixed for good. That could easily backfire if the automatic rules are wrong; so since there are not many others active in this area it may be a good idea to tread a bit more carefully here.
I installed OpenBSD recently and found a few issues in our tests caused by limitations of that platform. Similarly I saw a few issues on 32-bit Linux. Expect some patches soonish.
Eventually I’d like to find the time to improve handling of comments with –function-context (to show the comment before a function as part of its context) and for giving users better control over meta data (like time stamps) in git archive.
If there wasn’t already multiple such teams at work then I’d answer “whatever users need – see Jakub’s survey”. :)
The test suite and in particular the perf part could be improved. The latter should indicate how reliable its results are. A statistically significant number of measurements should be made – three is probably not enough, but I’m really bad at statistics. A higher number of runs would necessitate a lower complexity of the tests to keep the runtime bearable, which might be a bit tricky.
In addition to (wall-clock and CPU) time it would be interesting to measure I/O and memory (allocations, peak usage).
It would be nice if tests could be run automatically against repos with different shapes and sizes, and if the results would then indicate the how git scales per operation (e.g. a graph showing that log scales linearly with the number of commits).
Or perhaps that’s all a bit much and it would be better to add a performance log or warnings for operations that take longer than a few milliseconds. And some place to aggregate them (voluntarily).
In short: Better insight into git’s performance profile and how it changes from version to version.
The hard-coded use of SHA1, I guess. Work is underway already, but the mentioned backward compatibility makes it tricky.
And similarly I wish we wouldn’t treat all-zero hashes specially. The odds of a real object having that hash value may be negligible, but I feel it’s untidy, like using 0 instead of NULL for a pointer..
Visual Studio Code supports Git out of the box and seems to be quite nice in general, but I only installed it this month. Apart from that I don’t use any Git-related tools.
Not really related, but my favorite developer tool of the last months is Matt Godbolt’s compiler explorer. It makes it really easy to check the effect of optimizations across different compilers and their versions, and share the result. E.g. here you can see gcc removing a NULL check based on the fact that calls of memcpy(3) with a NULL pointer are undefined. Fun!
Various
Light reading
Git tools and sites
This edition of Git Rev News was curated by Christian Couder <christian.couder@gmail.com>, Thomas Ferris Nicolaisen <tfnico@gmail.com>, Jakub Narębski <jnareb@gmail.com> and Markus Jansen <mja@jansen-preisler.de> with help from René Scharfe, Raman Gupta and Johannes Schindelin.