Bug 1751948 - Part 2: Add support for generated UNIFIED_SOURCES files, r=glandium

This patch adds support for generated files to be placed in
UNIFIED_SOURCES.  This will be used in part 3 to build .cpp files
generated by IPDL_SOURCES by adding them to IPDL's UNIFIED_SOURCES under
the hood. Using a unified build for IPDL files is important, as a
significant build time regression was observed locally when using
non-unified sources to build ipdl sources, due to the large number of
generated files.

Differential Revision: https://phabricator.services.mozilla.com/D137166
This commit is contained in:
Nika Layzell 2022-02-28 21:01:47 +00:00
parent 9e65eb911e
commit a7de25bcb9
10 changed files with 133 additions and 172 deletions

View File

@ -31,15 +31,15 @@ from mozbuild.frontend.data import (
FinalTargetPreprocessedFiles,
FinalTargetFiles,
GeneratedFile,
GeneratedSources,
GnProjectData,
HostLibrary,
HostGeneratedSources,
HostSources,
IPDLCollection,
LocalizedPreprocessedFiles,
LocalizedFiles,
SandboxedWasmLibrary,
SharedLibrary,
Sources,
StaticLibrary,
UnifiedSources,
XPIDLModule,
@ -155,6 +155,9 @@ class CommonBackend(BuildBackend):
self._handle_xpcom_collection(obj)
elif isinstance(obj, UnifiedSources):
if obj.generated_files:
self._handle_generated_sources(obj.generated_files)
# Unified sources aren't relevant to artifact builds.
if self.environment.is_artifact_build:
return True
@ -178,8 +181,9 @@ class CommonBackend(BuildBackend):
)
return False
elif isinstance(obj, (GeneratedSources, HostGeneratedSources)):
self._handle_generated_sources(obj.files)
elif isinstance(obj, (Sources, HostSources)):
if obj.generated_files:
self._handle_generated_sources(obj.generated_files)
return False
elif isinstance(obj, GeneratedFile):

View File

@ -42,9 +42,7 @@ from ..frontend.data import (
FinalTargetFiles,
FinalTargetPreprocessedFiles,
GeneratedFile,
GeneratedSources,
HostDefines,
HostGeneratedSources,
HostLibrary,
HostProgram,
HostRustProgram,
@ -70,7 +68,6 @@ from ..frontend.data import (
StaticLibrary,
TestManifest,
VariablePassthru,
WasmGeneratedSources,
WasmSources,
XPIDLModule,
)
@ -450,7 +447,7 @@ class RecursiveMakeBackend(MakeBackend):
# CommonBackend.
assert os.path.basename(obj.output_path) == "Makefile"
self._create_makefile(obj)
elif isinstance(obj, (Sources, GeneratedSources)):
elif isinstance(obj, Sources):
suffix_map = {
".s": "ASFILES",
".c": "CSRCS",
@ -460,60 +457,51 @@ class RecursiveMakeBackend(MakeBackend):
".S": "SSRCS",
}
variables = [suffix_map[obj.canonical_suffix]]
if isinstance(obj, GeneratedSources):
base = backend_file.objdir
cls = ObjDirPath
prefix = "!"
else:
base = backend_file.srcdir
cls = SourcePath
prefix = ""
for f in sorted(obj.files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)), backend_file
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
for files, base, cls, prefix in (
(obj.static_files, backend_file.srcdir, SourcePath, ""),
(obj.generated_files, backend_file.objdir, ObjDirPath, "!"),
):
for f in sorted(files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)),
backend_file,
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
self._compile_graph[mozpath.join(backend_file.relobjdir, "target-objects")]
elif isinstance(obj, (HostSources, HostGeneratedSources)):
elif isinstance(obj, HostSources):
suffix_map = {
".c": "HOST_CSRCS",
".mm": "HOST_CMMSRCS",
".cpp": "HOST_CPPSRCS",
}
variables = [suffix_map[obj.canonical_suffix]]
if isinstance(obj, HostGeneratedSources):
base = backend_file.objdir
cls = ObjDirPath
prefix = "!"
else:
base = backend_file.srcdir
cls = SourcePath
prefix = ""
for f in sorted(obj.files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)), backend_file
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
for files, base, cls, prefix in (
(obj.static_files, backend_file.srcdir, SourcePath, ""),
(obj.generated_files, backend_file.objdir, ObjDirPath, "!"),
):
for f in sorted(files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)),
backend_file,
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
self._compile_graph[mozpath.join(backend_file.relobjdir, "host-objects")]
elif isinstance(obj, (WasmSources, WasmGeneratedSources)):
elif isinstance(obj, WasmSources):
suffix_map = {".c": "WASM_CSRCS", ".cpp": "WASM_CPPSRCS"}
variables = [suffix_map[obj.canonical_suffix]]
if isinstance(obj, WasmGeneratedSources):
base = backend_file.objdir
cls = ObjDirPath
prefix = "!"
else:
base = backend_file.srcdir
cls = SourcePath
prefix = ""
for f in sorted(obj.files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)), backend_file
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
for files, base, cls, prefix in (
(obj.static_files, backend_file.srcdir, SourcePath, ""),
(obj.generated_files, backend_file.objdir, ObjDirPath, "!"),
):
for f in sorted(files):
p = self._pretty_path(
cls(obj._context, prefix + mozpath.relpath(f, base)),
backend_file,
)
for var in variables:
backend_file.write("%s += %s\n" % (var, p))
self._compile_graph[mozpath.join(backend_file.relobjdir, "target-objects")]
elif isinstance(obj, VariablePassthru):
# Sorted so output is consistent and we don't bump mtimes.

View File

@ -21,7 +21,6 @@ from mozpack.files import FileFinder
from .common import CommonBackend
from ..frontend.data import (
Defines,
GeneratedSources,
HostProgram,
HostSources,
Library,
@ -103,9 +102,6 @@ class VisualStudioBackend(CommonBackend):
elif isinstance(obj, HostSources):
self._add_sources(reldir, obj)
elif isinstance(obj, GeneratedSources):
self._add_sources(reldir, obj)
elif isinstance(obj, UnifiedSources):
# XXX we should be letting CommonBackend.consume_object call this
# for us instead.

View File

@ -12,7 +12,6 @@ from mozbuild.backend.common import CommonBackend
from mozbuild.frontend.data import (
ComputedFlags,
Sources,
GeneratedSources,
DirectoryTraversal,
PerSourceFlag,
VariablePassthru,
@ -67,7 +66,7 @@ class CompileDBBackend(CommonBackend):
if isinstance(obj, DirectoryTraversal):
self._envs[obj.objdir] = obj.config
elif isinstance(obj, (Sources, GeneratedSources)):
elif isinstance(obj, Sources):
# For other sources, include each source file.
for f in obj.files:
self._build_db_line(

View File

@ -1463,7 +1463,7 @@ VARIABLES = {
""",
),
"UNIFIED_SOURCES": (
ContextDerivedTypedList(SourcePath, StrictOrderingOnAppendList),
ContextDerivedTypedList(Path, StrictOrderingOnAppendList),
list,
"""Source code files that can be compiled together.

View File

@ -319,7 +319,7 @@ class WebIDLCollection(ContextDerived):
# from regular ones so tests bindings aren't shipped.
return list(
group_unified_files(
self.all_regular_cpp_basenames(),
sorted(self.all_regular_cpp_basenames()),
unified_prefix="UnifiedBindings",
unified_suffix="cpp",
files_per_unified_file=32,
@ -1088,20 +1088,28 @@ class JARManifest(ContextDerived):
class BaseSources(ContextDerived):
"""Base class for files to be compiled during the build."""
__slots__ = ("files", "canonical_suffix")
__slots__ = ("files", "static_files", "generated_files", "canonical_suffix")
def __init__(self, context, files, canonical_suffix):
def __init__(self, context, static_files, generated_files, canonical_suffix):
ContextDerived.__init__(self, context)
self.files = files
# Sorted so output is consistent and we don't bump mtimes, but always
# order generated files after static ones to be consistent across build
# environments, which may have different objdir paths relative to
# topsrcdir.
self.static_files = sorted(static_files)
self.generated_files = sorted(generated_files)
self.files = self.static_files + self.generated_files
self.canonical_suffix = canonical_suffix
class Sources(BaseSources):
"""Represents files to be compiled during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
def __init__(self, context, static_files, generated_files, canonical_suffix):
BaseSources.__init__(
self, context, static_files, generated_files, canonical_suffix
)
class PgoGenerateOnlySources(BaseSources):
@ -1110,42 +1118,25 @@ class PgoGenerateOnlySources(BaseSources):
These files are only used during the PGO generation phase."""
def __init__(self, context, files):
BaseSources.__init__(self, context, files, ".cpp")
class GeneratedSources(BaseSources):
"""Represents generated files to be compiled during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
BaseSources.__init__(self, context, files, [], ".cpp")
class HostSources(HostMixin, BaseSources):
"""Represents files to be compiled for the host during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
class HostGeneratedSources(HostMixin, BaseSources):
"""Represents generated files to be compiled for the host during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
def __init__(self, context, static_files, generated_files, canonical_suffix):
BaseSources.__init__(
self, context, static_files, generated_files, canonical_suffix
)
class WasmSources(BaseSources):
"""Represents files to be compiled with the wasm compiler during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
class WasmGeneratedSources(BaseSources):
"""Represents generated files to be compiled with the wasm compiler during the build."""
def __init__(self, context, files, canonical_suffix):
BaseSources.__init__(self, context, files, canonical_suffix)
def __init__(self, context, static_files, generated_files, canonical_suffix):
BaseSources.__init__(
self, context, static_files, generated_files, canonical_suffix
)
class UnifiedSources(BaseSources):
@ -1153,15 +1144,21 @@ class UnifiedSources(BaseSources):
__slots__ = ("have_unified_mapping", "unified_source_mapping")
def __init__(self, context, files, canonical_suffix, files_per_unified_file):
BaseSources.__init__(self, context, files, canonical_suffix)
def __init__(self, context, static_files, generated_files, canonical_suffix):
BaseSources.__init__(
self, context, static_files, generated_files, canonical_suffix
)
unified_build = context.config.substs.get(
"ENABLE_UNIFIED_BUILD", False
) or context.get("REQUIRES_UNIFIED_BUILD", False)
files_per_unified_file = (
context.get("FILES_PER_UNIFIED_FILE", 16) if unified_build else 1
)
self.have_unified_mapping = files_per_unified_file > 1
if self.have_unified_mapping:
# Sorted so output is consistent and we don't bump mtimes.
source_files = list(sorted(self.files))
# On Windows, path names have a maximum length of 255 characters,
# so avoid creating extremely long path names.
unified_prefix = context.relsrcdir
@ -1173,7 +1170,10 @@ class UnifiedSources(BaseSources):
unified_prefix = "Unified_%s_%s" % (suffix, unified_prefix)
self.unified_source_mapping = list(
group_unified_files(
source_files,
# NOTE: self.files is already (partially) sorted, and we
# intentionally do not re-sort it here to avoid a dependency
# on the build environment's objdir path.
self.files,
unified_prefix=unified_prefix,
unified_suffix=suffix,
files_per_unified_file=files_per_unified_file,

View File

@ -4,7 +4,6 @@
from __future__ import absolute_import, print_function, unicode_literals
import itertools
import logging
import os
import six
@ -31,12 +30,10 @@ from .data import (
FinalTargetFiles,
FinalTargetPreprocessedFiles,
GeneratedFile,
GeneratedSources,
GnProjectData,
ExternalStaticLibrary,
ExternalSharedLibrary,
HostDefines,
HostGeneratedSources,
HostLibrary,
HostProgram,
HostRustProgram,
@ -70,7 +67,6 @@ from .data import (
UnifiedSources,
VariablePassthru,
WasmDefines,
WasmGeneratedSources,
WasmSources,
XPCOMComponentManifests,
XPIDLModule,
@ -649,8 +645,6 @@ class TreeMetadataEmitter(LoggingMixin):
host_linkables = []
wasm_linkables = []
unified_build = context.config.substs.get("ENABLE_UNIFIED_BUILD", False)
def add_program(prog, var):
if var.startswith("HOST_"):
host_linkables.append(prog)
@ -1056,10 +1050,6 @@ class TreeMetadataEmitter(LoggingMixin):
context,
)
# UNIFIED_SOURCES only take SourcePaths, so there should be no
# generated source in here
assert not gen_sources["UNIFIED_SOURCES"]
no_pgo = context.get("NO_PGO")
no_pgo_sources = [f for f, flags in six.iteritems(all_flags) if flags.no_pgo]
if no_pgo:
@ -1094,23 +1084,20 @@ class TreeMetadataEmitter(LoggingMixin):
for a in alternatives:
canonicalized_suffix_map[a] = suffix
def canonical_suffix_for_file(f):
return canonicalized_suffix_map[mozpath.splitext(f)[1]]
# A map from moz.build variables to the canonical suffixes of file
# kinds that can be listed therein.
all_suffixes = list(suffix_map.keys())
varmap = dict(
SOURCES=(Sources, GeneratedSources, all_suffixes),
HOST_SOURCES=(HostSources, HostGeneratedSources, [".c", ".mm", ".cpp"]),
UNIFIED_SOURCES=(UnifiedSources, None, [".c", ".mm", ".m", ".cpp"]),
SOURCES=(Sources, all_suffixes),
HOST_SOURCES=(HostSources, [".c", ".mm", ".cpp"]),
UNIFIED_SOURCES=(UnifiedSources, [".c", ".mm", ".m", ".cpp"]),
)
# Only include a WasmSources or WasmGeneratedSources context if there
# are any WASM_SOURCES. (This is going to matter later because we inject
# an extra .c file to compile with the wasm compiler if, and only if,
# there are any WASM sources.)
# Only include a WasmSources context if there are any WASM_SOURCES.
# (This is going to matter later because we inject an extra .c file to
# compile with the wasm compiler if, and only if, there are any WASM
# sources.)
if sources["WASM_SOURCES"] or gen_sources["WASM_SOURCES"]:
varmap["WASM_SOURCES"] = (WasmSources, WasmGeneratedSources, [".c", ".cpp"])
varmap["WASM_SOURCES"] = (WasmSources, [".c", ".cpp"])
# Track whether there are any C++ source files.
# Technically this won't do the right thing for SIMPLE_PROGRAMS in
# a directory with mixed C and C++ source, but it's not that important.
@ -1119,46 +1106,43 @@ class TreeMetadataEmitter(LoggingMixin):
# Source files to track for linkables associated with this context.
ctxt_sources = defaultdict(lambda: defaultdict(list))
for variable, (klass, gen_klass, suffixes) in varmap.items():
allowed_suffixes = set().union(*[suffix_map[s] for s in suffixes])
# First ensure that we haven't been given filetypes that we don't
# recognize.
for f in itertools.chain(sources[variable], gen_sources[variable]):
ext = mozpath.splitext(f)[1]
if ext not in allowed_suffixes:
raise SandboxValidationError(
"%s has an unknown file type." % f, context
)
for srcs, cls in (
(sources[variable], klass),
(gen_sources[variable], gen_klass),
for variable, (klass, suffixes) in varmap.items():
# Group static and generated files by their canonical suffixes, and
# ensure we haven't been given filetypes that we don't recognize.
by_canonical_suffix = defaultdict(lambda: {"static": [], "generated": []})
for srcs, key in (
(sources[variable], "static"),
(gen_sources[variable], "generated"),
):
# Now sort the files to let groupby work.
sorted_files = sorted(srcs, key=canonical_suffix_for_file)
for canonical_suffix, files in itertools.groupby(
sorted_files, canonical_suffix_for_file
):
if canonical_suffix in (".cpp", ".mm"):
cxx_sources[variable] = True
elif canonical_suffix in (".s", ".S"):
self._asm_compile_dirs.add(context.objdir)
arglist = [context, list(files), canonical_suffix]
if variable.startswith("UNIFIED_"):
if (
unified_build is False
and context.get("REQUIRES_UNIFIED_BUILD", False) is False
):
arglist.append(1)
else:
arglist.append(context.get("FILES_PER_UNIFIED_FILE", 16))
obj = cls(*arglist)
srcs = list(obj.files)
if isinstance(obj, UnifiedSources) and obj.have_unified_mapping:
srcs = dict(obj.unified_source_mapping).keys()
ctxt_sources[variable][canonical_suffix] += sorted(srcs)
yield obj
for f in srcs:
canonical_suffix = canonicalized_suffix_map.get(
mozpath.splitext(f)[1]
)
if canonical_suffix not in suffixes:
raise SandboxValidationError(
"%s has an unknown file type." % f, context
)
by_canonical_suffix[canonical_suffix][key].append(f)
# Yield an object for each canonical suffix, grouping generated and
# static sources together to allow them to be unified together.
for canonical_suffix in sorted(by_canonical_suffix.keys()):
if canonical_suffix in (".cpp", ".mm"):
cxx_sources[variable] = True
elif canonical_suffix in (".s", ".S"):
self._asm_compile_dirs.add(context.objdir)
src_group = by_canonical_suffix[canonical_suffix]
obj = klass(
context,
src_group["static"],
src_group["generated"],
canonical_suffix,
)
srcs = list(obj.files)
if isinstance(obj, UnifiedSources) and obj.have_unified_mapping:
srcs = sorted(dict(obj.unified_source_mapping).keys())
ctxt_sources[variable][canonical_suffix] += srcs
yield obj
if ctxt_sources:
for linkable in linkables:

View File

@ -19,7 +19,6 @@ from mozbuild.frontend.data import (
Exports,
FinalTargetPreprocessedFiles,
GeneratedFile,
GeneratedSources,
HostProgram,
HostRustLibrary,
HostRustProgram,
@ -1338,7 +1337,9 @@ class TestEmitterBasic(unittest.TestCase):
self.assertIsInstance(flags, ComputedFlags)
self.assertEqual(len(objs), 6)
generated_sources = [o for o in objs if isinstance(o, GeneratedSources)]
generated_sources = [
o for o in objs if isinstance(o, Sources) and o.generated_files
]
self.assertEqual(len(generated_sources), 6)
suffix_map = {obj.canonical_suffix: obj for obj in generated_sources}
@ -1355,7 +1356,8 @@ class TestEmitterBasic(unittest.TestCase):
for suffix, files in expected.items():
sources = suffix_map[suffix]
self.assertEqual(
sources.files, [mozpath.join(reader.config.topobjdir, f) for f in files]
sources.generated_files,
[mozpath.join(reader.config.topobjdir, f) for f in files],
)
for f in files:

View File

@ -798,15 +798,6 @@ class TestGroupUnifiedFiles(unittest.TestCase):
for i, amount in enumerate(expected_amounts):
check_mapping(i, amount)
def test_unsorted_files(self):
unsorted_files = ["a%d.cpp" % i for i in range(11)]
sorted_files = sorted(unsorted_files)
mapping = list(group_unified_files(unsorted_files, "Unified", "cpp", 5))
self.assertEqual(mapping[0][1], sorted_files[0:5])
self.assertEqual(mapping[1][1], sorted_files[5:10])
self.assertEqual(mapping[2][1], sorted_files[10:])
class TestMisc(unittest.TestCase):
def test_pair(self):

View File

@ -1205,9 +1205,6 @@ def group_unified_files(files, unified_prefix, unified_suffix, files_per_unified
files, and determining which original source files go in which unified
file."""
# Make sure the input list is sorted. If it's not, bad things could happen!
files = sorted(files)
# Our last returned list of source filenames may be short, and we
# don't want the fill value inserted by zip_longest to be an
# issue. So we do a little dance to filter it out ourselves.