From 30ab248cf6fd385907732303eec4f0795e8dc62e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 9 May 2013 13:05:33 -0400 Subject: [PATCH] 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 | 15 +++++ dom/bindings/BindingGen.py | 64 +++++++++----------- dom/bindings/Codegen.py | 5 +- dom/bindings/Makefile.in | 107 +++++++++++++++++++++------------- dom/bindings/test/Makefile.in | 41 +++++++++---- js/src/config/rules.mk | 15 +++++ 6 files changed, 158 insertions(+), 89 deletions(-) diff --git a/config/rules.mk b/config/rules.mk index 9584939fdea..1f63061b4cb 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -1619,6 +1619,21 @@ 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 diff --git a/dom/bindings/BindingGen.py b/dom/bindings/BindingGen.py index 80a8a8ed473..bcb41d79dac 100644 --- a/dom/bindings/BindingGen.py +++ b/dom/bindings/BindingGen.py @@ -5,41 +5,25 @@ 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" @@ -48,13 +32,8 @@ def main(): 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') @@ -64,13 +43,26 @@ def main(): # 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() diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 8f265f88dde..fc82a0d666b 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -357,7 +357,10 @@ class CGList(CGThing): """ 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) diff --git a/dom/bindings/Makefile.in b/dom/bindings/Makefile.in index 7c5eb6a06ed..153742b6e9d 100644 --- a/dom/bindings/Makefile.in +++ b/dom/bindings/Makefile.in @@ -33,6 +33,15 @@ 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 \ @@ -42,6 +51,18 @@ globalgen_targets := \ 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)) \ @@ -50,6 +71,7 @@ CPPSRCS = \ CallbackObject.cpp \ DOMJSProxyHandler.cpp \ $(NULL) +endif LOCAL_INCLUDES += -I$(topsrcdir)/js/xpconnect/src \ -I$(topsrcdir)/js/xpconnect/wrappers \ @@ -79,11 +101,11 @@ LOCAL_INCLUDES += \ $(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 @@ -92,14 +114,6 @@ 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 := \ @@ -107,6 +121,7 @@ bindinggen_dependencies := \ Bindings.conf \ Configuration.py \ Codegen.py \ + ParserResults.pkl \ parser/WebIDL.py \ $(GLOBAL_DEPS) \ $(NULL) @@ -127,35 +142,20 @@ $(webidl_files): %: $(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): %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_cpp_files): .BindingGen + +# $(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 \ @@ -180,24 +180,52 @@ $(CACHE_DIR)/.done: @$(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) \ @@ -205,6 +233,5 @@ distclean:: $(binding_cpp_files) \ $(all_webidl_files) \ $(globalgen_targets) \ - ParserResults.pkl - -.PHONY: webidl-export + ParserResults.pkl \ + $(NULL) diff --git a/dom/bindings/test/Makefile.in b/dom/bindings/test/Makefile.in index 3e609b524f1..f15907c652e 100644 --- a/dom/bindings/test/Makefile.in +++ b/dom/bindings/test/Makefile.in @@ -47,6 +47,7 @@ bindinggen_dependencies := \ ../Bindings.conf \ ../Configuration.py \ ../Codegen.py \ + ../ParserResults.pkl \ ../parser/WebIDL.py \ ../Makefile \ $(GLOBAL_DEPS) \ @@ -89,19 +90,18 @@ endif # 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 \ @@ -110,3 +110,20 @@ check:: 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 diff --git a/js/src/config/rules.mk b/js/src/config/rules.mk index 9584939fdea..1f63061b4cb 100644 --- a/js/src/config/rules.mk +++ b/js/src/config/rules.mk @@ -1619,6 +1619,21 @@ 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