After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 689194 - jhbuild tinderbox slows down builds
jhbuild tinderbox slows down builds
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
Depends on:
Blocks:
 
 
Reported: 2012-11-28 02:23 UTC by Allison Karlitskaya (desrt)
Modified: 2012-12-09 11:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix performance bottleneck in jhbuild tinderbox (3.27 KB, patch)
2012-11-28 03:04 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2012-11-28 02:23:42 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.
Comment 1 Craig Keogh 2012-11-28 02:53:17 UTC
I agree, the code is bad. A rewrite is needed.
Comment 2 Allison Karlitskaya (desrt) 2012-11-28 03:04:50 UTC
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.
Comment 3 Allison Karlitskaya (desrt) 2012-11-28 03:05:52 UTC
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...
Comment 4 Allison Karlitskaya (desrt) 2012-12-05 06:27:17 UTC
ping?
Comment 5 Craig Keogh 2012-12-05 11:20:29 UTC
Thank you for the patch Ryan and I appreciate your work. I'll review this in the next few days.
Comment 6 Craig Keogh 2012-12-08 12:19:25 UTC
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.
Comment 7 Allison Karlitskaya (desrt) 2012-12-08 12:38:32 UTC
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 8 Craig Keogh 2012-12-09 11:38:16 UTC
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.