Created attachment 554786 [details] [diff] [review] Non-recursive build system prototype for js/src (64 bit linux only)
Sounds like Joey would be interested in this.
Created attachment 555009 [details] [diff] [review] Non-recursive build system prototype for js/src (32 and 64 bit linux only) 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.
Created attachment 558827 [details] [diff] [review] Non-recursive build system prototype for js/src (32 and 64 bit linux only) 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.
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?
This is a really old bug that is no longer relevant to the current JS engine build system.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.