GNOME Bugzilla – Bug 689194
jhbuild tinderbox slows down builds
Last modified: 2012-12-09 11:38:36 UTC
The jhbuild tinderbox frontend contains some exceptionally dreadful code. In cmd.py the code to read the stdin and stdout (and pretty-print them to the html log) sleeps for 0.1 seconds after reading only 1024 bytes. This effectively rate-limits builds to producing 10k of output per second. Webkit is pretty chatty and with parallel builds enabled it easily hits this limit (by almost an order of magnitude) on my machine... There are some other bugs in this code too... it's why we often see output in the html log like Checking for xyz... yes. on separate lines. I'm going to rewrite it.
I agree, the code is bad. A rewrite is needed.
Created attachment 230056 [details] [review] Fix performance bottleneck in jhbuild tinderbox The HTML logging for jhbuild tinderbox contained code that would read 1024 bytes of build stdout and then sleep for 0.1 seconds. This effectively rate-limited builds to producing 10k of output per second, which is easily overrun by chatty builds like WebKit on fast systems. This commit changes the buffer size to 10000 and eliminates the sleep entirely. It also reworks the handling of when an output line is printed, always waiting until a newline is seen, except at the very end of builds. This prevents this sort of output: checking for sys/types.h... yes that was often seen (on separate lines) in the logfiles before.
I managed to avoid a complete rewrite. Testing this, the WebKit build now saturates all my CPU cores and is producing a bit over 60kB/s of stdout output...
ping?
Thank you for the patch Ryan and I appreciate your work. I'll review this in the next few days.
I'm reviewing the testing the patch now. It's looking good so far. Note that pprint_output is used for both terminal and tinderbox. When building using 'jhbuild buildone libxml2' (terminal) and outside JHBuild I get: checking if gcc supports -c -o file.o... /usr/bin/rm: cannot remove `conftest*': No such file or directory yes In the tinderbox output I get: /usr/bin/rm: cannot remove `conftest*': No such file or directory checking if gcc supports -c -o file.o... yes But I'm not bothered by that. But it is a little interesting. One more thing I want to test is if the build pauses for user input. I'll get to that soon.
Interesting. The explanation for that is that the patch groups stdout and stderr separately, waiting for newlines. In this case we have: - stdout (no newline) - stderr (with newline) - stdout (newline) since the stderr sees the newline first, it prints first We could possibly flush the contents of a stream completely (with or without newline) if we see any input at all on the other one... it's a bit tricky though because even then there is no guarantee that we see them in the right order, but it would be more likely to get the right behaviour 99% of the time (whereas the current system is almost guaranteed to give the above result).
Comment on attachment 230056 [details] [review] Fix performance bottleneck in jhbuild tinderbox Thank you! Committed. JHBuild is broken if a module build requires user input. But that was broken before your patch, and your patch has no impact.
http://git.gnome.org/browse/jhbuild/commit/?id=302573d3e5ff4744b78a3b1e7b9618e2d026d558