Bug 861587. Rejigger the WebIDL binding build system to do all binding codegen in a single python process while still using our dependency tracking for bindings to minimize the number of bindings we try to regenerate. r=khuey
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 09 May 2013 13:05:33 -0400
changeset 138181 712d3684efe44cff7d6963ba3d8af0d58e80f802
parent 138180 671e9ced84bc448be1656225edd2a2447148b356
child 138182 59c279d93f8e268449e5153b1330e9d254ea4f8a
push id3752
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 17:21:10 +0000
treeherdermozilla-aurora@1580544aef0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs861587
milestone23.0a1
Bug 861587. Rejigger the WebIDL binding build system to do all binding codegen in a single python process while still using our dependency tracking for bindings to minimize the number of bindings we try to regenerate. r=khuey
config/rules.mk
dom/bindings/BindingGen.py
dom/bindings/Codegen.py
dom/bindings/Makefile.in
dom/bindings/test/Makefile.in
js/src/config/rules.mk
--- a/config/rules.mk
+++ b/config/rules.mk
@@ -1614,16 +1614,31 @@ ifneq (,$(MDDEPEND_FILES))
 ifdef .PYMAKE
 includedeps $(MDDEPEND_FILES)
 else
 include $(MDDEPEND_FILES)
 endif
 endif
 
 endif
+
+
+ifneq (,$(filter export,$(MAKECMDGOALS)))
+MDDEPEND_FILES		:= $(strip $(wildcard $(addprefix $(MDDEPDIR)/,$(EXTRA_EXPORT_MDDEPEND_FILES))))
+
+ifneq (,$(MDDEPEND_FILES))
+ifdef .PYMAKE
+includedeps $(MDDEPEND_FILES)
+else
+include $(MDDEPEND_FILES)
+endif
+endif
+
+endif
+
 #############################################################################
 
 -include $(topsrcdir)/$(MOZ_BUILD_APP)/app-rules.mk
 -include $(MY_RULES)
 
 #
 # Generate Emacs tags in a file named TAGS if ETAGS was set in $(MY_CONFIG)
 # or in $(MY_RULES)
--- a/dom/bindings/BindingGen.py
+++ b/dom/bindings/BindingGen.py
@@ -1,76 +1,68 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import os
 import cPickle
 from Configuration import Configuration
-from Codegen import CGBindingRoot
+from Codegen import CGBindingRoot, replaceFileIfChanged
 
-def generate_binding_header(config, outputprefix, srcprefix, webidlfile):
+def generate_binding_files(config, outputprefix, srcprefix, webidlfile):
     """
     |config| Is the configuration object.
     |outputprefix| is a prefix to use for the header guards and filename.
     """
 
-    filename = outputprefix + ".h"
-    depsname = ".deps/" + filename + ".pp"
+    depsname = ".deps/" + outputprefix + ".pp"
     root = CGBindingRoot(config, outputprefix, webidlfile)
-    with open(filename, 'wb') as f:
-        f.write(root.declare())
+    replaceFileIfChanged(outputprefix + ".h", root.declare())
+    replaceFileIfChanged(outputprefix + ".cpp", root.define())
+
     with open(depsname, 'wb') as f:
         # Sort so that our output is stable
-        f.write("\n".join(filename + ": " + os.path.join(srcprefix, x) for
-                          x in sorted(root.deps())))
-
-def generate_binding_cpp(config, outputprefix, srcprefix, webidlfile):
-    """
-    |config| Is the configuration object.
-    |outputprefix| is a prefix to use for the header guards and filename.
-    """
-
-    filename = outputprefix + ".cpp"
-    depsname = ".deps/" + filename + ".pp"
-    root = CGBindingRoot(config, outputprefix, webidlfile)
-    with open(filename, 'wb') as f:
-        f.write(root.define())
-    with open(depsname, 'wb') as f:
-        f.write("\n".join(filename + ": " + os.path.join(srcprefix, x) for
+        f.write("\n".join(outputprefix + ": " + os.path.join(srcprefix, x) for
                           x in sorted(root.deps())))
 
 def main():
-
     # Parse arguments.
     from optparse import OptionParser
     usagestring = "usage: %prog [header|cpp] configFile outputPrefix srcPrefix webIDLFile"
     o = OptionParser(usage=usagestring)
     o.add_option("--verbose-errors", action='store_true', default=False,
                  help="When an error happens, display the Python traceback.")
     (options, args) = o.parse_args()
 
-    if len(args) != 5 or (args[0] != "header" and args[0] != "cpp"):
-        o.error(usagestring)
-    buildTarget = args[0]
-    configFile = os.path.normpath(args[1])
-    outputPrefix = args[2]
-    srcPrefix = os.path.normpath(args[3])
-    webIDLFile = os.path.normpath(args[4])
+    configFile = os.path.normpath(args[0])
+    srcPrefix = os.path.normpath(args[1])
 
     # Load the parsing results
     f = open('ParserResults.pkl', 'rb')
     parserData = cPickle.load(f)
     f.close()
 
     # Create the configuration data.
     config = Configuration(configFile, parserData)
 
-    # Generate the prototype classes.
-    if buildTarget == "header":
-        generate_binding_header(config, outputPrefix, srcPrefix, webIDLFile);
-    elif buildTarget == "cpp":
-        generate_binding_cpp(config, outputPrefix, srcPrefix, webIDLFile);
+    def readFile(f):
+        file = open(f, 'rb')
+        try:
+            contents = file.read()
+        finally:
+            file.close()
+        return contents
+    allWebIDLFiles = readFile(args[2]).split()
+    changedDeps = readFile(args[3]).split()
+
+    if all(f.endswith("Binding") or f == "ParserResults.pkl" for f in changedDeps):
+        toRegenerate = filter(lambda f: f.endswith("Binding"), changedDeps)
+        toRegenerate = map(lambda f: f[:-len("Binding")] + ".webidl", toRegenerate)
     else:
-        assert False # not reached
+        toRegenerate = allWebIDLFiles
+
+    for webIDLFile in toRegenerate:
+        assert webIDLFile.endswith(".webidl")
+        outputPrefix = webIDLFile[:-len(".webidl")] + "Binding"
+        generate_binding_files(config, outputPrefix, srcPrefix, webIDLFile);
 
 if __name__ == '__main__':
     main()
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -352,17 +352,20 @@ static DOMIfaceAndProtoJSClass Interface
 
 class CGList(CGThing):
     """
     Generate code for a list of GCThings.  Just concatenates them together, with
     an optional joiner string.  "\n" is a common joiner.
     """
     def __init__(self, children, joiner=""):
         CGThing.__init__(self)
-        self.children = children
+        # Make a copy of the kids into a list, because if someone passes in a
+        # generator we won't be able to both declare and define ourselves, or
+        # define ourselves more than once!
+        self.children = list(children)
         self.joiner = joiner
     def append(self, child):
         self.children.append(child)
     def prepend(self, child):
         self.children.insert(0, child)
     def join(self, generator):
         return self.joiner.join(filter(lambda s: len(s) > 0, (child for child in generator)))
     def declare(self):
--- a/dom/bindings/Makefile.in
+++ b/dom/bindings/Makefile.in
@@ -28,33 +28,55 @@ exported_binding_headers := $(subst .web
 # Set linked_binding_cpp_files before adding the test IDL to the mix
 linked_binding_cpp_files := $(subst .webidl,Binding.cpp,$(all_webidl_files))
 
 all_webidl_files += $(test_webidl_files)
 
 binding_header_files := $(subst .webidl,Binding.h,$(all_webidl_files))
 binding_cpp_files := $(subst .webidl,Binding.cpp,$(all_webidl_files))
 
+# We want to be able to only regenerate the .cpp and .h files that really need
+# to change when a .webidl file changes.  We do this by making the
+# binding_dependency_trackers targets have dependencies on the right .webidl
+# files via generated .pp files, having a .BindingGen target that depends on the
+# binding_dependency_trackers and which has all the generated binding .h/.cpp
+# depending on it, and then in the make commands for that target being able to
+# check which exact binding_dependency_trackers changed.
+binding_dependency_trackers := $(subst .webidl,Binding,$(all_webidl_files))
+
 globalgen_targets := \
   PrototypeList.h \
   RegisterBindings.h \
   RegisterBindings.cpp \
   UnionTypes.h \
   UnionTypes.cpp \
   UnionConversions.h \
   $(NULL)
 
+# Nasty hack: when the test/Makefile.in invokes us to do codegen, it
+# uses a target of
+# "export TestExampleInterface-example TestExampleProxyInterface-example".
+# We don't actually need to load our .o.pp files in that case, so just
+# pretend like we have no CPPSRCS if that's the target.  It makes the
+# test cycle much faster, which is why we're doing it.
+#
+# XXXbz We could try to cheat even more and only include our CPPSRCS
+# when $(MAKECMDGOALS) contains libs, so that we can skip loading all
+# those .o.pp when trying to make a single .cpp file too, but that
+# would break |make FooBinding.o(bj)|.  Ah, well.
+ifneq (export TestExampleInterface-example TestExampleProxyInterface-example,$(MAKECMDGOALS))
 CPPSRCS = \
   $(linked_binding_cpp_files) \
   $(filter %.cpp, $(globalgen_targets)) \
   BindingUtils.cpp \
   CallbackInterface.cpp \
   CallbackObject.cpp \
   DOMJSProxyHandler.cpp \
   $(NULL)
+endif
 
 LOCAL_INCLUDES += -I$(topsrcdir)/js/xpconnect/src \
   -I$(topsrcdir)/js/xpconnect/wrappers \
   -I$(topsrcdir)/content/canvas/src \
   -I$(topsrcdir)/content/html/content/src \
   -I$(topsrcdir)/media/webrtc/signaling/src/peerconnection \
   -I$(topsrcdir)/dom/base \
   -I$(topsrcdir)/dom/battery \
@@ -74,44 +96,37 @@ LOCAL_INCLUDES += \
 endif
 
 ifdef MOZ_B2G_RIL
 LOCAL_INCLUDES += \
   -I$(topsrcdir)/dom/icc/src \
   $(NULL)
 endif
 
-EXTRA_MDDEPEND_FILES := $(addsuffix .pp,$(binding_cpp_files) $(binding_header_files))
+EXTRA_EXPORT_MDDEPEND_FILES := $(addsuffix .pp,$(binding_dependency_trackers))
 
 EXPORTS_GENERATED_FILES := $(exported_binding_headers)
 EXPORTS_GENERATED_DEST := $(DIST)/include/$(binding_include_path)
-EXPORTS_GENERATED_TARGET := webidl-export
+EXPORTS_GENERATED_TARGET := export
 INSTALL_TARGETS += EXPORTS_GENERATED
 
 ifdef GNU_CC
 CXXFLAGS += -Wno-uninitialized
 endif
 
 include $(topsrcdir)/config/rules.mk
 
-# edmorley is sick of clobbering everytime someone adds an interface
-$(CPPOBJS): PrototypeList.h
-
-# We need to create a separate target so we can ensure that the pickle is
-# done before generating headers.
-export:: ParserResults.pkl
-	$(MAKE) webidl-export
-
 # If you change bindinggen_dependencies here, change it in
 # dom/bindings/test/Makefile.in too.
 bindinggen_dependencies := \
   BindingGen.py \
   Bindings.conf \
   Configuration.py \
   Codegen.py \
+  ParserResults.pkl \
   parser/WebIDL.py \
   $(GLOBAL_DEPS) \
   $(NULL)
 
 CSS2Properties.webidl: $(topsrcdir)/layout/style/nsCSSPropList.h \
                        $(topsrcdir)/layout/style/nsCSSPropAliasList.h \
                        $(webidl_base)/CSS2Properties.webidl.in \
                        $(webidl_base)/CSS2PropertiesProps.h \
@@ -122,45 +137,30 @@ CSS2Properties.webidl: $(topsrcdir)/layo
 	  $(srcdir)/GenerateCSS2PropertiesWebIDL.py $(webidl_base)/CSS2Properties.webidl.in > CSS2Properties.webidl
 
 $(webidl_files): %: $(webidl_base)/%
 	$(INSTALL) $(IFLAGS1) $(webidl_base)/$* .
 
 $(test_webidl_files): %: $(srcdir)/test/%
 	$(INSTALL) $(IFLAGS1) $(srcdir)/test/$* .
 
-$(binding_header_files): %Binding.h: $(bindinggen_dependencies) \
-                                     %.webidl \
-                                     $(call mkdir_deps,$(MDDEPDIR)) \
-                                     $(NULL)
-	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
-	  $(PLY_INCLUDE) -I$(srcdir)/parser \
-	  $(srcdir)/BindingGen.py header \
-	  $(srcdir)/Bindings.conf \
-	  $*Binding \
-	  $(CURDIR)/ \
-	  $*.webidl
+$(binding_header_files): .BindingGen
+
+$(binding_cpp_files): .BindingGen
 
-$(binding_cpp_files): %Binding.cpp: $(bindinggen_dependencies) \
-                                    $(CURDIR)/%.webidl \
-                                    $(call mkdir_deps,$(MDDEPDIR)) \
-                                    $(NULL)
-	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
-	  $(PLY_INCLUDE) -I$(srcdir)/parser \
-	  $(srcdir)/BindingGen.py cpp \
-	  $(srcdir)/Bindings.conf \
-	  $*Binding \
-	  $(CURDIR) \
-	  $*.webidl
+# $(binding_dependency_trackers) pick up additional dependencies via .pp files
+$(binding_dependency_trackers):
+	# Just bring it up to date, if it's out of date, so that we'll know that
+	# we have to redo binding generation and flag this prerequisite there as
+	# being newer than the bindinggen target.
+	@$(TOUCH) $@
 
 $(globalgen_targets): ParserResults.pkl
 
-%-example: $(bindinggen_dependencies) \
-           ParserResults.pkl \
-           $(NULL)
+%-example: .BindingGen
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
 	  $(PLY_INCLUDE) -I$(srcdir)/parser \
 	  $(srcdir)/ExampleGen.py \
 	  $(srcdir)/Bindings.conf $*
 
 CACHE_DIR = _cache
 
 globalgen_dependencies := \
@@ -175,36 +175,63 @@ globalgen_dependencies := \
   $(GLOBAL_DEPS) \
   $(NULL)
 
 $(CACHE_DIR)/.done:
 	$(MKDIR) -p $(CACHE_DIR)
 	@$(TOUCH) $@
 
 ParserResults.pkl: $(globalgen_dependencies)
+	# Running GlobalGen.py updates ParserResults.pkl as a side-effect
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
     $(PLY_INCLUDE) -I$(srcdir)/parser \
     $(srcdir)/GlobalGen.py $(srcdir)/Bindings.conf . \
     --cachedir=$(CACHE_DIR) \
     $(all_webidl_files)
 
+.BindingGen: $(bindinggen_dependencies) $(binding_dependency_trackers)
+	# Make sure .deps actually exists, since we'll try to write to it from
+	# BindingGen.py but we're typically running in the export phase, which
+	# is before anyone has bothered creating .deps.
+	$(MKDIR) -p .deps
+        # Pass our long lists through files to try to avoid blowing
+        # out the command line
+	echo $(all_webidl_files) > .all-webidl-file-list
+	echo $? > .changed-dependency-list
+	# BindingGen.py will examine the changed dependency list to figure out
+	# what it really needs to regenerate.
+	$PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
+    $(PLY_INCLUDE) -I$(srcdir)/parser \
+    $(srcdir)/BindingGen.py \
+    $(srcdir)/Bindings.conf \
+    $(CURDIR) \
+    .all-webidl-file-list \
+    .changed-dependency-list
+	# Now touch the .BindingGen file so that we don't have to keep redoing
+	# all that until something else actually changes.
+	@$(TOUCH) $@
+
 GARBAGE += \
   webidlyacc.py \
   parser.out \
   $(wildcard *-example.h) \
   $(wildcard *-example.cpp) \
+  .BindingGen \
+  .all-webidl-file-list \
+  .changed-dependency-list \
+  $(binding_dependency_trackers) \
   $(NULL)
 
 # Make sure all binding header files are created during the export stage, so we
 # don't have issues with .cpp files being compiled before we've generated the
 # headers they depend on.  This is really only needed for the test files, since
-# the non-test headers are all exported above anyway.
-webidl-export:: $(binding_header_files)
+# the non-test headers are all exported above anyway.  Note that this means that
+# we do all of our codegen during export.
+export:: $(binding_header_files)
 
 distclean::
 	-$(RM) \
         $(binding_header_files) \
         $(binding_cpp_files) \
         $(all_webidl_files) \
         $(globalgen_targets) \
-        ParserResults.pkl
-
-.PHONY: webidl-export
+        ParserResults.pkl \
+        $(NULL)
--- a/dom/bindings/test/Makefile.in
+++ b/dom/bindings/test/Makefile.in
@@ -42,16 +42,17 @@ LOCAL_INCLUDES += \
 # If you change bindinggen_dependencies here, change it in
 # dom/bindings/Makefile.in too.  But note that we include ../Makefile
 # here manually, since $(GLOBAL_DEPS) won't cover it.
 bindinggen_dependencies := \
   ../BindingGen.py \
   ../Bindings.conf \
   ../Configuration.py \
   ../Codegen.py \
+  ../ParserResults.pkl \
   ../parser/WebIDL.py \
   ../Makefile \
   $(GLOBAL_DEPS) \
   $(NULL)
 
 MOCHITEST_FILES := \
   test_bug773326.html \
   test_enums.html \
@@ -84,29 +85,45 @@ MOCHITEST_CHROME_FILES = \
 ifdef GNU_CC
 CXXFLAGS += -Wno-uninitialized
 endif
 
 # Include rules.mk before any of our targets so our first target is coming from
 # rules.mk and running make with no target in this dir does the right thing.
 include $(topsrcdir)/config/rules.mk
 
-$(CPPSRCS): ../%Binding.cpp: $(bindinggen_dependencies) \
-                             ../%.webidl \
-                             TestExampleInterface-example \
-                             TestExampleProxyInterface-example \
-                             $(NULL)
-	$(MAKE) -C .. $*Binding.h
-	$(MAKE) -C .. $*Binding.cpp
+$(CPPSRCS): .BindingGen
 
-TestExampleInterface-example:
-	$(MAKE) -C .. TestExampleInterface-example
-
-TestExampleProxyInterface-example:
-	$(MAKE) -C .. TestExampleProxyInterface-example
+.BindingGen: $(bindinggen_dependencies) \
+             $(test_webidl_files) \
+             $(NULL)
+	# The export phase in dom/bindings is what actually looks at
+	# dependencies and regenerates things as needed, so just go ahead and
+	# make that phase here.  Also make our example interface files.  If the
+	# target used here ever changes, change the conditional around
+	# $(CPPSRCS) in dom/bindings/Makefile.in.
+	$(MAKE) -C .. export TestExampleInterface-example TestExampleProxyInterface-example
+	@$(TOUCH) $@
 
 check::
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
 	  $(PLY_INCLUDE) $(srcdir)/../parser/runtests.py
 
 check-interactive:
 	PYTHONDONTWRITEBYTECODE=1 $(PYTHON) $(topsrcdir)/config/pythonpath.py \
 	  $(PLY_INCLUDE) $(srcdir)/../parser/runtests.py -q
+
+# Since we define MOCHITEST_FILES, config/makefiles/mochitest.mk goes ahead and
+# sets up a rule with libs:: in itm which makes our .DEFAULT_TARGET be "libs".
+# Then ruls.mk does |.DEFAULT_TARGET ?= default| which leaves it as "libs".  So
+# if we make without an explicit target in this directory, we try to make
+# "libs", but with a $(MAKECMDGOALS) of empty string.  And then rules.mk
+# helpfully does not include our *.o.pp files, since it includes them only if
+# filtering some stuff out from $(MAKECMDGOALS) leaves it nonempty.  The upshot
+# is that if some headers change and we run make in this dir without an explicit
+# target things don't get rebuilt.
+#
+# On the other hand, if we set .DEFAULT_TARGET to "default" explicitly here,
+# then rules.mk will reinvoke make with "export" and "libs" but this time hey
+# will be passed as explicit targets, show up in $(MAKECMDGOALS), and things
+# will work.  Do this at the end of our Makefile so the rest of the build system
+# does not get a chance to muck with it after we set it.
+.DEFAULT_GOAL := default
--- a/js/src/config/rules.mk
+++ b/js/src/config/rules.mk
@@ -1614,16 +1614,31 @@ ifneq (,$(MDDEPEND_FILES))
 ifdef .PYMAKE
 includedeps $(MDDEPEND_FILES)
 else
 include $(MDDEPEND_FILES)
 endif
 endif
 
 endif
+
+
+ifneq (,$(filter export,$(MAKECMDGOALS)))
+MDDEPEND_FILES		:= $(strip $(wildcard $(addprefix $(MDDEPDIR)/,$(EXTRA_EXPORT_MDDEPEND_FILES))))
+
+ifneq (,$(MDDEPEND_FILES))
+ifdef .PYMAKE
+includedeps $(MDDEPEND_FILES)
+else
+include $(MDDEPEND_FILES)
+endif
+endif
+
+endif
+
 #############################################################################
 
 -include $(topsrcdir)/$(MOZ_BUILD_APP)/app-rules.mk
 -include $(MY_RULES)
 
 #
 # Generate Emacs tags in a file named TAGS if ETAGS was set in $(MY_CONFIG)
 # or in $(MY_RULES)