SERVER-45724 Make new generator as fast as old module

4 files changed, 124 insertions, 29 deletions

diff --git a/SConstruct b/SConstruct
index f65a4add436..e3ac11cb54c 100644
--- a/SConstruct
+++ b/SConstruct
@@ -3817,6 +3817,20 @@ if get_option('ninja') == 'true':
     env.NinjaRegisterFunctionHandler("integration_test_list_builder_action", skip)
     env.NinjaRegisterFunctionHandler("benchmark_list_builder_action", skip)
 
+
+    # idlc.py has the ability to print it's implicit dependencies
+    # while generating, Ninja can consume these prints using the
+    # deps=msvc method.
+    env.AppendUnique(IDLCFLAGS= "--write-dependencies-inline")
+    env.NinjaRule(
+        rule="IDLC",
+        command="cmd /c $cmd" if env.TargetOSIs("windows") else "$cmd",
+        description="Generating $out",
+        deps="msvc",
+    )
+    env.NinjaRuleMapping("$IDLCCOM", "IDLC")
+    env.NinjaRuleMapping(env["IDLCCOM"], "IDLC")
+
     # We can create empty files for FAKELIB in Ninja because it
     # does not care about content signatures. We have to
     # write_uuid_to_file for FAKELIB in SCons because SCons does.
diff --git a/site_scons/site_tools/idl_tool.py b/site_scons/site_tools/idl_tool.py
index 03b4254e417..d8888521e7c 100755
--- a/site_scons/site_tools/idl_tool.py
+++ b/site_scons/site_tools/idl_tool.py
@@ -37,8 +37,20 @@ def idlc_emitter(target, source, env):
         )
 
     base_file_name, _ = SCons.Util.splitext(str(target[0]))
-    target_source = base_file_name + "_gen.cpp"
-    target_header = base_file_name + "_gen.h"
+    target_source = env.File(base_file_name + "_gen.cpp")
+    target_header = env.File(base_file_name + "_gen.h")
+
+    # When generating ninja we need to inform each IDL build the
+    # string it should search for to find implicit dependencies.
+    if env.get("GENERATING_NINJA", False):
+        setattr(target_source.attributes, "NINJA_EXTRA_VARS", {"msvc_deps_prefix": "import file:"})
+        setattr(target_header.attributes, "NINJA_EXTRA_VARS", {"msvc_deps_prefix": "import file:"})
+
+        # IDL can generate too-long commands on Windows and does not
+        # need environment variables, so disable them by pre-setting
+        # NINJA_ENV_ENV to an empty string.
+        setattr(target_source.attributes, "NINJA_ENV_ENV", "")
+        setattr(target_header.attributes, "NINJA_ENV_ENV", "")
 
     env.Alias("generated-sources", [target_source, target_header])
 
@@ -49,6 +61,13 @@ IDLCAction = SCons.Action.Action("$IDLCCOM", "$IDLCCOMSTR")
 
 
 def idl_scanner(node, env, path):
+
+    # When generating ninja we only need to add the IDL_GLOBAL_DEPS
+    # because the implicit dependencies will be picked up using the
+    # deps=msvc method.
+    if env.get("GENERATING_NINJA", False):
+        return IDL_GLOBAL_DEPS
+
     nodes_deps_list = getattr(node.attributes, "IDL_NODE_DEPS", None)
     if nodes_deps_list is not None:
         return nodes_deps_list
@@ -95,12 +114,13 @@ def generate(env):
     idlc = idlc_mod
 
     env["IDLC"] = "$PYTHON buildscripts/idl/idlc.py"
-    env["IDLCFLAGS"] = ""
-    base_dir = env.subst("$BUILD_ROOT/$VARIANT_DIR").replace("#", "")
-    env["IDLCCOM"] = (
-        "$IDLC --include src --base_dir %s --target_arch $TARGET_ARCH --header ${TARGETS[1]} --output ${TARGETS[0]} $SOURCES "
-        % (base_dir)
-    )
+    base_dir = env.Dir("$BUILD_ROOT/$VARIANT_DIR").path
+    env["IDLCFLAGS"] = [
+        "--include", "src",
+        "--base_dir", base_dir,
+        "--target_arch", "$TARGET_ARCH",
+    ]
+    env["IDLCCOM"] = "$IDLC $IDLCFLAGS --header ${TARGETS[1]} --output ${TARGETS[0]} $SOURCES"
     env["IDLCSUFFIX"] = ".idl"
 
     IDL_GLOBAL_DEPS = env.Glob("#buildscripts/idl/*.py") + env.Glob(
diff --git a/site_scons/site_tools/ninja.py b/site_scons/site_tools/ninja.py
index 56fadf1cc5a..0fa84a89665 100644
--- a/site_scons/site_tools/ninja.py
+++ b/site_scons/site_tools/ninja.py
@@ -113,10 +113,7 @@ def alias_to_ninja_build(node):
 
 def get_dependencies(node):
     """Return a list of dependencies for node."""
-    return [
-        get_path(src_file(child))
-        for child in node.children()
-    ]
+    return [get_path(src_file(child)) for child in node.children()]
 
 
 def get_inputs(node):
@@ -593,6 +590,23 @@ class NinjaState:
                 # other builds.
                 build["order_only"] = "_generated_sources"
 
+            # When using a depfile Ninja can only have a single output
+            # but SCons will usually have emitted an output for every
+            # thing a command will create because it's caching is much
+            # more complex than Ninja's. This includes things like DWO
+            # files. Here we make sure that Ninja only ever sees one
+            # target when using a depfile. It will still have a command
+            # that will create all of the outputs but most targets don't
+            # depend direclty on DWO files and so this assumption is safe
+            # to make.
+            rule = self.rules.get(build["rule"])
+
+            # Some rules like 'phony' and other builtins we don't have
+            # listed in self.rules so verify that we got a result
+            # before trying to check if it has a deps key.
+            if rule is not None and rule.get("deps"):
+                build["outputs"] = build["outputs"][0:1]
+
             ninja.build(**build)
 
         template_builds = dict()
@@ -807,19 +821,8 @@ def get_command(env, node, action):  # pylint: disable=too-many-branches
             cmd = cmd[0:-2].strip()
 
     outputs = get_outputs(node)
-    if rule == "CMD_W_DEPS":
-        # When using a depfile Ninja can only have a single output but
-        # SCons will usually have emitted an output for every thing a
-        # command will create because it's caching is much more
-        # complex than Ninja's. This includes things like DWO
-        # files. Here we make sure that Ninja only ever sees one
-        # target when using a depfile. It will still have a command
-        # that will create all of the outputs but most targets don't
-        # depend direclty on DWO files and so this assumption is
-        # safe to make.
-        outputs = outputs[0:1]
-
-    command_env = getattr(node.attributes, "NINJA_ENV_ENV", "")
+    command_env = getattr(node.attributes, "NINJA_ENV_ENV", None)
+
     # If win32 and rule == CMD_W_DEPS then we don't want to calculate
     # an environment for this command. It's a compile command and
     # compiledb doesn't support shell syntax on Windows. We need the
@@ -829,8 +832,11 @@ def get_command(env, node, action):  # pylint: disable=too-many-branches
     #
     # On POSIX we can still set environment variables even for compile
     # commands so we do so.
-    if not command_env and not (env["PLATFORM"] == "win32" and rule == "CMD_W_DEPS"):
+    if command_env is None and not (
+        env["PLATFORM"] == "win32" and rule == "CMD_W_DEPS"
+    ):
         ENV = get_default_ENV(sub_env)
+        command_env = ""
 
         # This is taken wholesale from SCons/Action.py
         #
@@ -856,13 +862,20 @@ def get_command(env, node, action):  # pylint: disable=too-many-branches
                 command_env += "{}={} ".format(key, value)
 
         setattr(node.attributes, "NINJA_ENV_ENV", command_env)
+    elif command_env is None:
+        command_env = ""
+
+    variables = {"cmd": command_env + cmd}
+    extra_vars = getattr(node.attributes, "NINJA_EXTRA_VARS", {})
+    if extra_vars:
+        variables.update(extra_vars)
 
     ninja_build = {
         "outputs": outputs,
         "inputs": get_inputs(node),
         "implicit": implicit,
         "rule": rule,
-        "variables": {"cmd": command_env + cmd},
+        "variables": variables,
     }
 
     # Don't use sub_env here because we require that NINJA_POOL be set
@@ -968,13 +981,18 @@ def register_custom_rule_mapping(env, pre_subst_string, rule):
     __NINJA_RULE_MAPPING[pre_subst_string] = rule
 
 
-def register_custom_rule(env, rule, command, description=""):
+def register_custom_rule(env, rule, command, description="", deps=None):
     """Allows specification of Ninja rules from inside SCons files."""
-    env[NINJA_RULES][rule] = {
+    rule_obj = {
         "command": command,
         "description": description if description else "{} $out".format(rule),
     }
 
+    if deps is not None:
+        rule_obj["deps"] = deps
+
+    env[NINJA_RULES][rule] = rule_obj
+
 
 def register_custom_pool(env, pool, size):
     """Allows the creation of custom Ninja pools"""
@@ -1070,6 +1088,20 @@ def ninja_whereis(thing, *_args, **_kwargs):
         return path
 
 
+def ninja_always_serial(self, num, taskmaster):
+    """Replacement for SCons.Job.Jobs constructor which always uses the Serial Job class."""
+    # We still set self.num_jobs to num even though it's a lie. The
+    # only consumer of this attribute is the Parallel Job class AND
+    # the Main.py function which instantiates a Jobs class. It checks
+    # if Jobs.num_jobs is equal to options.num_jobs, so if the user
+    # provides -j12 but we set self.num_jobs = 1 they get an incorrect
+    # warning about this version of Python not supporting parallel
+    # builds. So here we lie so the Main.py will not give a false
+    # warning to users.
+    self.num_jobs = num
+    self.job = SCons.Job.Serial(taskmaster)
+
+
 class NinjaEternalTempFile(SCons.Platform.TempFileMunge):
     """Overwrite the __call__ method of SCons' TempFileMunge to not delete."""
 
@@ -1213,6 +1245,10 @@ def generate(env):
     if not exists(env):
         return
 
+    # Set a known variable that other tools can query so they can
+    # behave correctly during ninja generation.
+    env["GENERATING_NINJA"] = True
+
     # These methods are no-op'd because they do not work during ninja
     # generation, expected to do no work, or simply fail. All of which
     # are slow in SCons. So we overwrite them with no logic.
@@ -1226,6 +1262,11 @@ def generate(env):
     # symlinks which we're not producing.
     SCons.Node.FS.LocalFS.lstat = ninja_noop
 
+    # This is a slow method that isn't memoized. We make it a noop
+    # since during our generation we will never use the results of
+    # this or change the results.
+    SCons.Node.FS.is_up_to_date = ninja_noop
+
     # We overwrite stat and WhereIs with eternally memoized
     # implementations. See the docstring of ninja_stat and
     # ninja_whereis for detailed explanations.
@@ -1262,6 +1303,13 @@ def generate(env):
     # call.
     env["PRINT_CMD_LINE_FUNC"] = ninja_print
 
+    # This reduces unnecessary subst_list calls to add the compiler to
+    # the implicit dependencies of targets. Since we encode full paths
+    # in our generated commands we do not need these slow subst calls
+    # as executing the command will fail if the file is not found
+    # where we expect it.
+    env["IMPLICIT_COMMAND_DEPENDENCIES"] = False
+
     # Set build to no_exec, our sublcass of FunctionAction will force
     # an execution for ninja_builder so this simply effects all other
     # Builders.
@@ -1271,6 +1319,11 @@ def generate(env):
     # SConsign file.
     env.SetOption("max_drift", 1)
 
+    # The Serial job class is SIGNIFICANTLY (almost twice as) faster
+    # than the Parallel job class for generating Ninja files. So we
+    # monkey the Jobs constructor to only use the Serial Job class.
+    SCons.Job.Jobs.__init__ = ninja_always_serial
+
     # We will eventually need to overwrite TempFileMunge to make it
     # handle persistent tempfiles or get an upstreamed change to add
     # some configurability to it's behavior in regards to tempfiles.
diff --git a/site_scons/site_tools/thin_archive.py b/site_scons/site_tools/thin_archive.py
index cbc0cd03890..e563669a7eb 100644
--- a/site_scons/site_tools/thin_archive.py
+++ b/site_scons/site_tools/thin_archive.py
@@ -68,11 +68,19 @@ def _add_scanner(builder):
 
     def new_scanner(node, env, path):
         old_results = old_scanner(node, env, path)
+
+        # Ninja uses only timestamps for implicit dependencies so will
+        # always rebuild a program whose archive has been updated even
+        # if has the same content signature.
+        if env.get("GENERATING_NINJA", False):
+            return old_results
+
         new_results = []
         for base in old_results:
             new_results.append(base)
             if getattr(env.Entry(base).attributes, "thin_archive", None):
                 new_results.extend(base.children())
+
         return new_results
 
     builder.target_scanner = SCons.Scanner.Scanner(