Last Comment Bug 970567 - make distcc builds work for both HOST and TARGET compilation
: make distcc builds work for both HOST and TARGET compilation
Status: RESOLVED FIXED
[c=tools p=1 s= u=]
: perf
Product: Firefox OS
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: 2.0 S5 (4july)
Assigned To: Dave Huseby [:huseby]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-02-10 14:10 PST by Dave Huseby [:huseby]
Modified: 2014-07-03 15:56 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch to force make to use fully qualified paths to compiler executables when USE_DISTCC is defined (53 bytes, text/x-github-pull-request)
2014-02-10 14:18 PST, Dave Huseby [:huseby]
mwu: review+
vicamo: feedback+
Details
a patch to enable distcc for v1.3t tarako builds (53 bytes, text/x-github-pull-request)
2014-06-19 17:10 PDT, Dave Huseby [:huseby]
no flags Details

Description Dave Huseby [:huseby] 2014-02-10 14:10:33 PST
Currently the TARGET portion of the b2g build uses relative paths to the cross-compiling toolchain.  This prevents distcc from working because it can only handle generic executable calls resolved via search path (e.g. "gcc -c foo.c") and full path executable calls (e.g. "/opt/gcc -c foo.c").  With relative paths, the remote distcc hosts can't find the correct executable to run.
Comment 1 Dave Huseby [:huseby] 2014-02-10 14:18:16 PST
Created attachment 8373642 [details]
patch to force make to use fully qualified paths to compiler executables when USE_DISTCC is defined
Comment 2 Fabrice Desré [:fabrice] 2014-02-10 23:24:31 PST
Comment on attachment 8373642 [details]
patch to force make to use fully qualified paths to compiler executables when USE_DISTCC is defined

I'll let mwu and vicamo review.
Comment 3 Michael Wu [:mwu] 2014-02-11 11:26:24 PST
We should make a TARGET_TOOLCHAIN_ROOT variable and run realpath on that, and then use it to generate TARGET_TOOLS_PREFIX.
Comment 4 Vicamo Yang [:vicamo][:vyang] 2014-02-11 23:26:28 PST
(In reply to Michael Wu [:mwu] from comment #3)
> We should make a TARGET_TOOLCHAIN_ROOT variable and run realpath on that,
> and then use it to generate TARGET_TOOLS_PREFIX.

But that doesn't work on host toolchain.  For HOST_darwin-x86 and HOST_linux-x86, HOST_{CC,CXX} are aliased to $(CC) and $(CXX) directly.  For HOST_windows-x86, "TOOLS_PREFIX" is used but it's still assigned to empty string by now.  Creating a TARGET_TOOLCHAIN_ROOT, and maybe HOST_TOOLCHAIN_ROOT as well, means you have to touch many many files just like what we had in that pull request.

So why not call $(realpath $($(combo_target)CC)) directly?  "build/core/combo/select.mk" is used to select both TARGET_ and HOST_ mk files.  One can easily get absolute path for those variables by:

  ifneq ($(USE_DISTCC),)
    $(combo_target)CC := $(realpath $($(combo_target)CC))
    $(combo_target)CXX := $(realpath $($(combo_target)CXX))
  endif

in four lines.  Just four lines and you get it for all platforms, both host and target.
Comment 5 Dave Huseby [:huseby] 2014-02-13 11:43:13 PST
Let me rework this patch to include your suggestions and see if that works.  Thanks for the feedback.
Comment 6 Dave Huseby [:huseby] 2014-02-14 16:38:20 PST
Comment on attachment 8373642 [details]
patch to force make to use fully qualified paths to compiler executables when USE_DISTCC is defined

I updated the pull request to reflect the feedback.
Comment 7 Dave Huseby [:huseby] 2014-02-14 16:42:03 PST
I updated the patch to just modify select.mk as suggested in the bugzilla bug. This is all that is needed in the platform_build to make distcc work correctly. The user still needs to have a .userconfig that has something like this in it:

export CC=gcc-4.6
export CXX=g++-4.6
export USE_DISTCC=${USE_DISTCC:-}

# enable distcc if specified
if [ "${USE_DISTCC}" == "1" ]; then
  export CC=`which ${CC}`
  export CXX=`which ${CXX}`
  export CCACHE_PREFIX="distcc"
fi

This will make sure that both CC and CXX have full static path's to the 4.6 compiler on their machine as well as set distcc to be the "prefix command" that ccache uses when calling either compiler. The prefix command settings is essential to making ccache and distcc work well together. 

My results are pretty dramatic. I have a rMBP with a quad-core i7 in it. A full rebuild from scratch on that machine alone takes 45 minutes. I also have a dual quad-core Xeon workstation that I used for distcc and with it in the pool, the full rebuild time drops to 18 mintues. When bisecting bugs in gecko, the combination of ccache and distcc drops the build times to well under 5 minutes. I've seen several builds of only ~1m45s.
Comment 8 Vicamo Yang [:vicamo][:vyang] 2014-02-14 21:22:00 PST
Comment on attachment 8373642 [details]
patch to force make to use fully qualified paths to compiler executables when USE_DISTCC is defined

Not a Gonk peer, so I place only f+.
Comment 9 Dave Huseby [:huseby] 2014-03-05 09:49:21 PST
merged: http://bit.ly/MNMXPl

NOTE: this patch does NOT work with the new 4.3 and 4.4 branches of platform_build.  That means it cannot be pulled into the nexus-4 and nexus-4-kk builds as-is.  I was seeing build errors in libicuuc.
Comment 10 Dave Huseby [:huseby] 2014-06-19 17:10:55 PDT
Created attachment 8443149 [details]
a patch to enable distcc for v1.3t tarako builds

the first patch for this bug never made it into v1.3t for tarako builds.  this new patch fixes that.
Comment 11 Dave Huseby [:huseby] 2014-06-19 17:11:16 PDT
re-opening to get the v1.3t patch reviewed and landed.
Comment 12 Vicamo Yang [:vicamo][:vyang] 2014-06-19 18:42:31 PDT
Comment on attachment 8443149 [details]
a patch to enable distcc for v1.3t tarako builds

I think the thing you really need is to ask for 1.3T approval.
Comment 13 Michael Wu [:mwu] 2014-06-22 18:08:45 PDT
Comment on attachment 8443149 [details]
a patch to enable distcc for v1.3t tarako builds

I think it's too late to ask for things on 1.3t unless they're showstoppers.
Comment 14 Dave Huseby [:huseby] 2014-06-24 10:31:20 PDT
alrighty, I'll just keep it as a patch I apply locally.

Note You need to log in before you can comment on or make changes to this bug.


Privacy Policy