Closed Bug 680824 Opened 13 years ago Closed 8 years ago

Prototype for de-recursifying js/src's build system

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: sandervv, Assigned: sandervv)

References

Details

Attachments

(1 file, 2 obsolete files)

Non-recursive build system prototype for js/src (32 and 64 bit linux only)
23.06 KB, patch
Details | Diff | Splinter Review
During the last few days, I've been working on a prototype build system. The main purpose is faster (re)builds. This prototype is currently only targeted at the JavaScript engine (js/src), but the concept can be applied to all other components as well. The current state of this build system is that it successfully builds the JavaScript shell and libmozjs for a Linux 64 bit architecture. Design goals of this prototype: =============================== 1. Faster (re)builds (improved parallelism, just one Make process). 2. Easy to maintain the build system. Root Makefile (in this prototype called "js/src/fast.mk") includes all Rules.mk files from the subdirs. 3. Object files are added on a per-directory level. 4. Specify CFLAGS for object files per-directory level. 5. Deleting "obj-linux" will remove all generated files (e.g. object files and generated header files). Thus, there are no more object files intermixed with the source files. Overview of the build system: =============================== Per directory Rules.mk files (including js/src itself): |-- Rules.mk |-- assembler | |-- Rules.mk | |-- assembler | | |-- Rules.mk | |-- jit | | `-- Rules.mk | `-- wtf | |-- Rules.mk |-- config | |-- Rules.mk |-- editline | |-- Rules.mk |-- frontend | |-- Rules.mk |-- jsapi-tests | |-- Rules.mk |-- methodjit | |-- Rules.mk [...] |-- vm | |-- Rules.mk |-- yarr | |-- Rules.mk The rest of the build system is stored in: - js/src/fast.mk (includes all Rules.mk files from the subdirs). - js/src/build/common.mk (common logic to build object files from c++ files). The main Makefile is currently called "fast.mk", since I wanted to test/benchmark both build systems. This allows having both build systems in a single tree and makes it easier for someone else to test the non-recursive build system (since it doesn't modify the existing build system). I chose "Rules.mk", since there is a file "config/rules.mk" already. The file "js/src/build/common.mk" is used in almost every Rules.mk, and it eliminates redundant lines in Rules.mk. As mentioned earlier, all object files and generated files are stored in "js/src/obj-linux". Currently, the object files are intermixed with the source files. IMHO, I think it is better to separate generated content from editable source files. Benchmarks (http://pastebin.mozilla.org/1307752): =============================== Machine: Gentoo amd64, i7, 8 GB ram; build: -Os. | Original | Non-recursive | Complete rebuild | 30 sec | 26 sec | "touch jsbool.h" | 25 sec | 22 sec | "touch jsreflect.h" | 6.7 sec | 4.3 sec | Usage of non-recursive build system: =============================== As mentioned above, this prototype is only tested on a 64 bit Linux box. After applying the attached patch, the following instruction should build the JavaScript engine using the non-recursive build system: cd js/src ./configure make -ffast.mk -j12 Similar to the Kernel's build system, I also added a more silent mode: make -ffast.mk -j12 SILENT=1 This is similar to the current build system's output of "make -s", but does also display the progress of linking libmozjs and other binaries. Currently, SILENT is an environment variable, but this could be added to the configure-script to enable it by default (and make it toggle-able from the command line using SILENT=0). TODO list of this prototype (IMHO, ordered on most useful to do): =============================== 1. remove redundant CFLAGS from all Rules.mk. 2. better integration with the files generated from the existing ./configure script (e.g. integrate more variables from build/config.mk in fast.mk and in all Rules.mk files). 3. support linux 32 bit architecture. 4. start working on support for other platforms and architectures. In order to keep the patch and still be able to build the whole source tree, I noticed that check-sync-dirs reports that the build-dirs mismatch. To fix this, use: ln -s {../js/src/,}build/common.mk ln -s {../js/src/,}config/Rules.mk Later on, I hope that this prototype concept can be used to re-engineer time-consuming parts of the Firefox source tree. I'm looking forward to feedback about improvement of this build system and feedback of testers.
Sounds like Joey would be interested in this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Related: bug 623617
See Also: → 623617
Compared to the first patch, this patch contains the following improvements: 1. remove redundant CFLAGS from all Rules.mk files. 2. better integration with the files generated from the existing ./configure script (e.g. integrate more variables from build/config.mk in fast.mk and in all Rules.mk files). 3. support linux 32 bit architecture. So, the last item of the TODO list remains: 4. start working on support for other platforms and architectures. Currently, I'm requesting Level 1 access to the try server, which will help testing the different platforms and architecture. The refactoring of this patch allows using a general template for Rules.mk file: include build/header.mk # Rules for <Component> OBJ_$(d) := \ $(b)/<file0>.o \ $(b)/<file1>.o \ $(b)/<file2>.o \ include build/common.mk include build/footer.mk The variables $(d) and $(b) are set in build/header.mk and there value is respectively the relative path to the source directory and the build directory. Besides the OBJ_$(d) variable, it is also common to add libmozjs.so dependencies: OBJ_LIBMOZJS += $(OBJ_$(d)) and directory-specific CFLAGS: CFLAGS_$(d) += -I. -I./assembler -I./nanojit -I./tracejit Currently, there is no CXXFLAGS_$(d). The variable CFLAGS_$(d) is used for both CXX and CC invocation.
Attachment #554786 - Attachment is obsolete: true
(In reply to Sander Mathijs van Veen from comment #4) > Currently, I'm requesting Level 1 access to the try server, which will help > testing the different platforms and architecture. In the meantime if you wish me to send things to try for you, I'm more than happy to do so. Either ask here, ping me via email or ask me (edmorley) on #pymake/#developers.
Updated the patch to support m-c's current tip. See also the design goals mentioned in the comments above. I tested the patch on 32 and 64 bit Gentoo Linux: cd js/src ./configure make -ffast.mk -j12 SILENT=1 THIN_ARCHIVE=1 Note: both environment variables should be moved into the configure script. This patch does not affect the original build system. Results: ----------------------------------------- non-recursive build system smvv@multivac ~/moz/mozilla-central/js/src $ make clean -s; rm -rf dist; ccache -C; time make -ffast.mk -sj12 THIN_ARCHIVE=1 real 0m40.928s user 4m37.840s sys 0m17.494s smvv@multivac ~/moz/mozilla-central/js/src $ touch jsreflect.h smvv@multivac ~/moz/mozilla-central/js/src $ time make -ffast.mk -sj12 THIN_ARCHIVE=1 real 0m2.884s user 0m4.626s sys 0m0.775s ----------------------------------------- original build system smvv@multivac ~/moz/mozilla-central/js/src $ make clean -s; rm -rf dist; ccache -C; time make -sj12 real 0m45.960s user 4m34.661s sys 0m18.422s smvv@multivac ~/moz/mozilla-central/js/src $ touch jsreflect.h smvv@multivac ~/moz/mozilla-central/js/src $ time make -sj12 jsreflect.cpp js.cpp real 0m6.470s user 0m4.477s sys 0m1.335s ----------------------------------------- The THIN_ARCHIVE environment variable is a minor optimization for libjs_static.a. It adds the "T" flag to "ar"; instead of copying all object files into the archive, it creates a symbol index and references to the object files. See also this part of the ar man page: GNU ar can optionally create a thin archive, which contains a symbol index and references to the original copies of the member files of the archives. Such an archive is useful for building libraries for use within a local build, where the relocatable objects are expected to remain available, and copying the contents of each object would only waste time and space. Thin archives are also flattened, so that adding one or more archives to a thin archive will add the elements of the nested archive individually. The paths to the elements of the archive are stored relative to the archive itself. It saves approx. 1 second per rebuild/incremental build. What are the disadvantages of using thin archives? ----------------------------------------- In order to analysis the build system, I created a simple script and web app to visualize the build system's behavior. The analysis of the non-recursive build system can be found at: http://smvv.kompiler.org/bsa/ Clicking on the bars shows the execve() system calls in the top window. If there are sub processes, the bar will expand and show the sub processes. The analysis is a waterfall graph and shows clearly that the non-recursive build system is good at parallelism. On almost every interval, 12 jobs are running in parallel. At the end of the graph, the linking is done and shows underutilisation of the remaining cores. I think that using the gold linker as well, the build times will improve even further. Sidenote: The script uses strace and I think that logging each execve/vfork etc. system call has a negative impact on the timing results. In the waterfall graph, the top Make process' duration is 50 seconds, while without strace, the non-recursive build system needs only 40 seconds to complete its task. This is work in progress.
Assignee: nobody → sandervv
Attachment #555009 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #558827 - Flags: feedback?(khuey)
Note that we already have our own home-grown static library replacement, implemented in bug 584474. We generate $(LIBRARY).desc files that simply contain lists of object file names, and unwrap them before passing them to the linker. (js_static.a is still being generated because of the "DIST_INSTALL = 1" in js/src/Makefile.in, we can probably fix that.)
I have a bunch of comments here, but I haven't gotten through the whole thing, so those probably won't get posted till tomorrow. Sorry about that :-/
Comment on attachment 558827 [details] [diff] [review] Non-recursive build system prototype for js/src (32 and 64 bit linux only) Review of attachment 558827 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I took so long :-/ Looks pretty good, some general comments: 1) To land it we will of course need to replace all the hardcoded stuff with stuff that grabs the various variables directly from autoconf.mk. 2) Lets try to avoid target specific variables. Pymake has different propagation behavior there. 3) The dependency logic probably needs to be more complex. I remember there being cases that the simple include case doesn't catch (which escape me at the moment). I think we can land this in the tree if we can get to a place where it works correctly on one platform (presumably Linux), honoring stuff from configure/etc, and it's designed in such a way that doesn't box us in in the future (e.g. it's not obviously incompatible with windows/pymake/etc) ::: js/src/Rules.mk @@ +2,5 @@ > + > +# Rules for JS engine > + > +LIB_MOZJS := $(BUILD_DIR)/libmozjs.so > +AR_JS_STATIC := $(BUILD_DIR)/libjs_static.a Obviously these values are only good for Unix. We'll want to grab the right stuff out of autoconf.mk. @@ +16,5 @@ > + $(b)/include/jsautooplen.h \ > + $(b)/include/jsautokw.h \ > + $(b)/include/jsautocfg.h \ > + > +OBJ_LIBMOZJS += \ I'd rather list cpp files, than object files (mostly because the extension doesn't change across platforms). @@ +81,5 @@ > +include build/common.mk > + > +$(OBJ_$(d)): COMPILE_CXXFLAGS += -I$(BUILD_DIR) -I./assembler -I./nanojit \ > + -I./tracejit -DEXPORT_JS_API -DUSE_SYSTEM_MALLOC=1 -DMOZILLA_CLIENT \ > + -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1 I'd try to avoid target specific variables like this if at all possible. Pymake doesn't propagate those down (in other words, COMPILE_CXXFLAGS won't be set for any prerequisites for $(OBJ_$(d))) because the behavior is racy. @@ +123,5 @@ > +system_wrappers_js: $(SYSTEM_WRAPPERS_JS)/unix.h | $(b)/system_wrappers_js > + > +$(SYSTEM_WRAPPERS_JS)/%.h: $(d)/config/system-headers $(d)/config/make-system-wrappers.pl > + $(PYTHON) ./config/Preprocessor.py -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux -DHAVE_64BIT_OS=1 -DD_INO=d_ino -DJS_CPU_X64=1 -DJS_PUNBOX64=1 -DJS_METHODJIT=1 -DJS_MONOIC=1 -DJS_POLYIC=1 -DJS_POLYIC_TYPED_ARRAY=1 -DFEATURE_NANOJIT=1 -DJS_TRACER=1 -DAVMPLUS_AMD64=1 -DAVMPLUS_64BIT=1 -DAVMPLUS_UNIX=1 -DAVMPLUS_LINUX=1 -DSTDC_HEADERS=1 -DHAVE_SSIZE_T=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DJS_HAVE_STDINT_H=1 -DJS_BYTES_PER_WORD=8 -DJS_BITS_PER_WORD_LOG2=6 -DJS_ALIGN_OF_POINTER=8 -DJS_BYTES_PER_DOUBLE=8 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_UINT=1 -DHAVE_UNAME_DOMAINNAME_FIELD=1 -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1 -DHAVE_VISIBILITY_ATTRIBUTE=1 -DHAVE_DIRENT_H=1 -DHAVE_GETOPT_H=1 -DHAVE_SYS_BITYPES_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_GNU_LIBC_VERSION_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_MALLOC_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_STATFS_H=1 -DHAVE_SYS_VFS_H=1 -DHAVE_SYS_MOUNT_H=1 -DHAVE_SYS_QUOTA_H=1 -DHAVE_LINUX_QUOTA_H=1 -DHAVE_MMINTRIN_H=1 -DNEW_H=\<new\> -DHAVE_SYS_CDEFS_H=1 -DHAVE_DLOPEN=1 -DHAVE_DLADDR=1 -D_REENTRANT=1 -DHAVE_FCHMOD=1 -DHAVE_FLOCKFILE=1 -DHAVE_GETC_UNLOCKED=1 -DHAVE_GETPAGESIZE=1 -DHAVE_LCHOWN=1 -DHAVE_LOCALTIME_R=1 -DHAVE_LSTAT64=1 -DHAVE_MEMMOVE=1 -DHAVE_RANDOM=1 -DHAVE_SBRK=1 -DHAVE_SNPRINTF=1 -DHAVE_STAT64=1 -DHAVE_STATVFS=1 -DHAVE_STATVFS64=1 -DHAVE_STRERROR=1 -DHAVE_STRTOK_R=1 -DHAVE_TRUNCATE64=1 -DHAVE_CLOCK_MONOTONIC=1 -DHAVE_WCRTOMB=1 -DHAVE_MBRTOWC=1 -DHAVE_RES_NINIT=1 -DHAVE_GNU_GET_LIBC_VERSION=1 -DHAVE_ICONV=1 -DVA_COPY=va_copy -DHAVE_VA_COPY=1 -DHAVE_VA_LIST_AS_ARRAY=1 -DHAVE_CPP_EXPLICIT=1 -DHAVE_CPP_TYPENAME=1 -DHAVE_CPP_MODERN_SPECIALIZE_TEMPLATE_SYNTAX=1 -DHAVE_CPP_PARTIAL_SPECIALIZATION=1 -DHAVE_CPP_ACCESS_CHANGING_USING=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_NAMESPACE_STD=1 -DHAVE_CPP_UNAMBIGUOUS_STD_NOTEQUAL=1 -DHAVE_CPP_NEW_CASTS=1 -DHAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR=1 -DNEED_CPP_UNUSED_IMPLEMENTATIONS=1 -DHAVE_THREAD_TLS_KEYWORD=1 -DMALLOC_H=\<malloc.h\> -DHAVE_STRNDUP=1 -DHAVE_POSIX_MEMALIGN=1 -DHAVE_MEMALIGN=1 -DHAVE_VALLOC=1 -DHAVE_I18N_LC_MESSAGES=1 -DHAVE_LOCALECONV=1 -DNS_ALWAYS_INLINE=__attribute__\(\(always_inline\)\) -DNS_ATTR_MALLOC=__attribute__\(\(malloc\)\) -DNS_WARN_UNUSED_RESULT=__attribute__\(\(warn_unused_result\)\) -DNS_NORETURN=__attribute__\(\(noreturn\)\) -DMOZ_DEBUG_SYMBOLS=1 -DJSGC_TESTPILOT=1 -DHAVE___CXA_DEMANGLE=1 -DHAVE__UNWIND_BACKTRACE=1 -DHAVE_TM_ZONE_TM_GMTOFF=1 -DCPP_THROW_NEW=throw\(\) -DEDITLINE=1 -DMOZ_DLL_SUFFIX=\".so\" -DHAVE_POSIX_FALLOCATE=1 -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DHAVE_SETLOCALE=1 -DHAVE_LOCALECONV=1 \ > + $< | $(PERL) ./config/make-system-wrappers.pl $(@D) Obviously we wouldn't want to ship this with that hardcoded list ;-) ::: js/src/assembler/Rules.mk @@ +3,5 @@ > +# Rules for Assembler > + > +OBJ_$(d) := > + > +include build/footer.mk Is this file actually needed or is this just a placeholder? ::: js/src/build/header.mk @@ +1,4 @@ > +sp := $(sp).x > +dirstack_$(sp) := $(d) > +d := $(dir) > +b := $(BUILD_DIR)/$(d) Comments would be great here. It looks like d is the directory in the objdir, and b is the directory in the srcdir? ::: js/src/editline/Rules.mk @@ +10,5 @@ > + > +$(OBJ_$(d)): COMPILE_CFLAGS += -DANSI_ARROWS -DHAVE_TCGETATTR -DHIDE \ > + -DUSE_DIRENT -DSYS_UNIX -DHAVE_STDLIB -DUNIQUE_HISTORY > + > +$(b)/%.o: $(d)/%.c $(SYSTEM_WRAPPERS_JS)/unix.h This is the kind of thing we'll want to split out into its own variable (e.g. EXTRA_DEPS_$(d) = $(SYSTEM_WRAPPERS_JS)/unix.h or something similar). ::: js/src/fast.mk @@ +75,5 @@ > +include $(dir)/Rules.mk > + > +# TODO: implement Nitro assembler detection > +dir := assembler > +include $(dir)/Rules.mk This is the kind of thing that screams "write a macro". ::: js/src/methodjit/Rules.mk @@ +4,5 @@ > + > +# TODO: replace this expression with all filename. > +OBJ_$(d) := $(patsubst $(d)/%.cpp,$(b)/%.o,$(wildcard $(d)/*.cpp)) > + > +OBJ_LIBMOZJS += $(OBJ_$(d)) I'm a little confused by all this. We're doing object files for .cpps but source files for assembly?
Attachment #558827 - Flags: feedback?(khuey)
This is a really old bug that is no longer relevant to the current JS engine build system.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: