Skip to content

Commit

Permalink
Optimize CcInfo computation (#4020)
Browse files Browse the repository at this point in the history
This shows up around 4% of buildbuddy analysis phase
<img width="1728" alt="image"
src="https://github.com/user-attachments/assets/18344d22-ea21-4dbf-8542-2cf2763fa2ba">

After this change, it goes away.

It seems like Bazel is able to make this lazy, which is good since it's
only used for building LINKMODE_C_ARCHIVE/LINKMODE_C_SHARED which is
fairly rare.

**What type of PR is this?**
Performance improvement

**What does this PR do? Why is it needed?**
Avoid computing CcInfo for every intermediate library when it's not
needed

**Which issues(s) does this PR fix?**

Fixes #

**Other notes for review**
  • Loading branch information
dzbarsky authored Aug 16, 2024
1 parent aa96a11 commit a01ba7c
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 303 deletions.
74 changes: 37 additions & 37 deletions docs/go/core/rules.md

Large diffs are not rendered by default.

6 changes: 0 additions & 6 deletions go/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,6 @@ def as_tuple(v):
return tuple(v.to_list())
fail("as_tuple failed on {}".format(v))

_STRUCT_TYPE = type(struct())

def is_struct(v):
"""Returns true if v is a struct."""
return type(v) == _STRUCT_TYPE

def count_group_matches(v, prefix, suffix):
"""Counts reluctant substring matches between prefix and suffix.
Expand Down
18 changes: 1 addition & 17 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ load(
"COVERAGE_OPTIONS_DENYLIST",
"GO_TOOLCHAIN",
"as_iterable",
"is_struct",
)
load(
":mode.bzl",
Expand Down Expand Up @@ -282,7 +281,6 @@ def _library_to_source(go, attr, library, coverage_instrumented):
"cxxopts": _expand_opts(go, "cxxopts", getattr(attr, "cxxopts", [])),
"clinkopts": _expand_opts(go, "clinkopts", getattr(attr, "clinkopts", [])),
"cgo_exports": [],
"cc_info": None,
"pgoprofile": getattr(attr, "pgoprofile", None),
}

Expand Down Expand Up @@ -310,11 +308,10 @@ def _library_to_source(go, attr, library, coverage_instrumented):
# sources. compilepkg will catch these instead.
if f.extension in ("c", "cc", "cxx", "cpp", "hh", "hpp", "hxx"):
fail("source {} has C/C++ extension, but cgo was not enabled (set 'cgo = True')".format(f.path))

if library.resolve:
library.resolve(go, attr, source, _merge_embed)

source["cc_info"] = _collect_cc_infos(attr_deps + generated_deps, source["cdeps"])

return GoSource(**source)

def _collect_runfiles(go, data, deps):
Expand All @@ -327,19 +324,6 @@ def _collect_runfiles(go, data, deps):
[get_source(t).runfiles for t in deps],
)

def _collect_cc_infos(deps, cdeps):
cc_infos = []
for dep in cdeps:
if CcInfo in dep:
cc_infos.append(dep[CcInfo])
for dep in deps:
# dep may be a struct, which doesn't support indexing by providers.
if is_struct(dep):
continue
if GoSource in dep:
cc_infos.append(dep[GoSource].cc_info)
return cc_common.merge_cc_infos(cc_infos = cc_infos)

def _check_binary_dep(go, dep, edge):
"""Checks that this rule doesn't depend on a go_binary or go_test.
Expand Down
2 changes: 1 addition & 1 deletion go/private/providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# A represenatation of the inputs to a go package.
# A representation of the inputs to a go package.
# This is a configuration independent provider.
# You must call resolve with a mode to produce a GoSource.
# See go/providers.rst#GoLibrary for full documentation.
Expand Down
Loading

0 comments on commit a01ba7c

Please sign in to comment.