Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop tool to measure efficiency impact of configuration transitions #10613

Open
gregestren opened this issue Jan 17, 2020 · 5 comments
Open
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request

Comments

@gregestren
Copy link
Contributor

Tracking issue on Bazel Configurability Roadmap

As a precursor to optimizing multiplatform builds with trimming and better action caching, we need to understand how much inefficiency there currently is and where it comes from.

This issue covers a tool you can run over a project to describe this information as precisely as possible. Beside helping optimize today's builds, this provides clearer guidance for ongoing trimming/output caching efforts.

@gregestren gregestren added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Jan 17, 2020
@gregestren gregestren self-assigned this Jan 17, 2020
@gregestren gregestren added the P1 I'll work on this now. (Assignee required) label Jan 17, 2020
bazel-io pushed a commit that referenced this issue Jan 30, 2020
…cisely.

Before this change, any reliance on --define a=foo reports a dependency on the
"CoreOptions" fragment. While true, that's too broad: it includes *all*
--defines, including ones having nothing to do with "a".

This change reports --define like user-defined flags (which it essentially
is a special-case version of): return the specific --define instead. In the
above example that'd be --define:a (using a syntax that's easy enough to read
while being reasonably parseable as a no-spaces token).

This only works for select(). It doesn't include MAKE variables, e.g.:
https://github.com/bazelbuild/bazel/blob/6bd6cc4435e722393127c72189dbb646149d844b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java#L610-L621. That
requires its own
analysis.

Prereq work for #10613.

PiperOrigin-RevId: 292350812
bazel-io pushed a commit that referenced this issue Feb 4, 2020
This also generally refactors ConfigCommand to make a clean separation between
structural logic and output formatting logic.

ConfigCommand is responsible for taking a raw BuildConfiguration and translating it
into appropriate data structures that represent what it wants to output: what data
is output, how data relates to other data, and how results are ordered.

An output formatter then consumes these data structures to style the output
accordingly.

This also moves the logic that handled both out of BuildConfiguration & FragmentOptions.

It's neat how simple this makes JsonOutputFormatter. :)

Serves #10613.

PiperOrigin-RevId: 293152292
bazel-io pushed a commit that referenced this issue Feb 5, 2020
Also uncovered some bugs in production logic. Go testing!

Prep work for #10613.

PiperOrigin-RevId: 293415172
bazel-io pushed a commit that referenced this issue Feb 5, 2020
bazel-io pushed a commit that referenced this issue Feb 6, 2020
bazel-io pushed a commit that referenced this issue Feb 6, 2020
Aspects can add new rule dependencies via implicit label
attributes (see
https://docs.bazel.build/versions/2.0.0/skylark/aspects.html#aspect-definition-1).
We need to make sure these are included in whatever the rule the aspect
attaches to.

Note that *native* aspects can also declare *direct* fragment dependencies (
Starlark aspects can't). This change doesn't handle that. TBD.

Supports #10613.

PiperOrigin-RevId: 293669124
bazel-io pushed a commit that referenced this issue Mar 16, 2020
This is necessary to link options to the fragment(s) that contain them.

For example, if two configurations have a different --linkopt value, this belongs to the CppOptions FragmentOptions, which is included with the CppConfiguration fragment.

Serves #10613.

Example output:

$ blaze config <config hash>
INFO: Displaying config with id cc19b5007
BuildConfiguration  cc19b5007
Skyframe Key: BuildConfigurationValue.Key[cc19b5007]
Fragments: com.google.devtools.build.lib.analysis.PlatformConfiguration: [com.google.devtools.build.lib.analysis.PlatformOptions], com.google.devtools.build.lib.analysis.ShellConfiguration:[com.google.devtools.build.lib.analysis.ShellConfiguration$Options], ...
FragmentOptions com.google.devtools.build.lib.analysis.PlatformOptions {
  enabled_toolchain_types: []
  experimental_add_exec_constraints_to_targets: []
  extra_toolchains: []
  incompatible_auto_configure_host_platform: false
  incompatible_use_toolchain_resolution_for_java_rules: true
  platform_mappings: tools/platform_mappings
...
PiperOrigin-RevId: 301158932
bazel-io pushed a commit that referenced this issue Apr 27, 2020
Any Make variable "$(foo)" is equivalent to requiring
"--define foo".

This works for native rule logic and Starlark rules routing
through https://docs.bazel.build/versions/master/skylark/lib/ctx.html#expand_make_variables,
but not Starlark rules routing through
https://docs.bazel.build/versions/master/skylark/lib/ctx.html#var.

Supports #10613.
Added refactoring TODO in #11221.

PiperOrigin-RevId: 308632503
@gregestren
Copy link
Contributor Author

P1 review: still actively being worked on, albeit ~15% of my total time. #11258 is the current next step.

gregestren added a commit to gregestren/bazel that referenced this issue Jun 9, 2020
gregestren added a commit to gregestren/bazel that referenced this issue Jun 9, 2020
gregestren added a commit to gregestren/bazel that referenced this issue Jul 9, 2020
bazel-io pushed a commit that referenced this issue Jul 13, 2020
Work toward #10613

Update license name

Add :srcs

Add imports attributes.

Change-Id: I21d43e4cf80871529b860f02bca7c3b6ccbace3e
gregestren added a commit to gregestren/bazel that referenced this issue Jul 14, 2020
i.e. first check-in of [Measuring Configuration Overhead](https://docs.google.com/document/d/10ZxO2wZdKJATnYBqAm22xT1k5r4Vp6QX96TkqSUIhs0/edit).

This just establishes supporting structure. The tool is not yet functional.

Specifically:
- `types.py`: defines data structures for "configuration" and "configured target"
- `bazel_api.py`: API to translate `bazel cquery` and `bazel config` calls into the above data structures
- `bazel_api_test.py`: tests
- `ctexplain.py`: stump of an entry point

The tests utilize an existing Python test framework for invoking Bazel (`//src/test/py/bazel:test_base`).

Work towards bazelbuild#10613

Closes bazelbuild#11511.

PiperOrigin-RevId: 315479921
Change-Id: I00898a4579e20483c8c4c3f5250afc22ee1e1695
bazel-io pushed a commit that referenced this issue Jul 15, 2020
i.e. first check-in of [Measuring Configuration Overhead](https://docs.google.com/document/d/10ZxO2wZdKJATnYBqAm22xT1k5r4Vp6QX96TkqSUIhs0/edit).

This just establishes supporting structure. The tool is not yet functional.

Specifically:
- `types.py`: defines data structures for "configuration" and "configured target"
- `bazel_api.py`: API to translate `bazel cquery` and `bazel config` calls into the above data structures
- `bazel_api_test.py`: tests
- `ctexplain.py`: stump of an entry point

The tests utilize an existing Python test framework for invoking Bazel (`//src/test/py/bazel:test_base`).

Work towards #10613

Closes #11511.

PiperOrigin-RevId: 321409588
bazel-io pushed a commit that referenced this issue Aug 6, 2020
Include the required fragments list even for null-configured targets.

In other words:

  //my:source_file (null)

becomes

  //my:source_file (null) []

For #10613

PiperOrigin-RevId: 325293531
@gregestren
Copy link
Contributor Author

gregestren commented Aug 20, 2020

For what it's worth I'm running a dev version of this over real projects and getting really interesting results.

Still going through the review process, which will take a few more PRs until the checked in version catches up. If anyone is interested in understanding performance effects of transitions, please ping.

bazel-io pushed a commit that referenced this issue Aug 25, 2020
#11511 set up basic project structure.  This PR adds minimum working functionality.

Specifically, you can run it with a build command and it reports basic stats on the build's graph.

Example:

```
$ bazel-bin/tools/ctexplain/ctexplain -b "//testapp:foo"
Collecting configured targets for //testapp:foo... done in 0.62 s.

Configurations: 3
Targets: 79
Configured targets: 92 (+16.5% vs. targets)
Targets with multiple configs: 13
```

Notes:
* Changed import structure to prefer module imports over function, class imports (style guide recommendation)
* Set up structure for injecting arbitrary analyses. Each analysis consumes the build's set of configured targets and can output whatever it wants.
* Implemented one basic analysis
* Structured code to make it easy to fork output formatters (e.g. for machine-readable output). But tried not to add speculative inheritance / boilerplate too soon

Context: [Measuring Configuration Overhead](https://docs.google.com/document/d/10ZxO2wZdKJATnYBqAm22xT1k5r4Vp6QX96TkqSUIhs0/edit).
Work towards #10613

Closes #11829.

PiperOrigin-RevId: 328325094
@gregestren gregestren added P2 We'll consider working on this in future. (Assignee optional) and removed P1 I'll work on this now. (Assignee required) labels Dec 9, 2020
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    This is necessary to link options to the fragment(s) that contain them.

    For example, if two configurations have a different --linkopt value, this belongs to the CppOptions FragmentOptions, which is included with the CppConfiguration fragment.

    Serves bazelbuild/bazel#10613.

    Example output:

    $ blaze config <config hash>
    INFO: Displaying config with id cc19b5007
    BuildConfiguration  cc19b5007
    Skyframe Key: BuildConfigurationValue.Key[cc19b5007]
    Fragments: com.google.devtools.build.lib.analysis.PlatformConfiguration: [com.google.devtools.build.lib.analysis.PlatformOptions], com.google.devtools.build.lib.analysis.ShellConfiguration:[com.google.devtools.build.lib.analysis.ShellConfiguration$Options], ...
    FragmentOptions com.google.devtools.build.lib.analysis.PlatformOptions {
      enabled_toolchain_types: []
      experimental_add_exec_constraints_to_targets: []
      extra_toolchains: []
      incompatible_auto_configure_host_platform: false
      incompatible_use_toolchain_resolution_for_java_rules: true
      platform_mappings: tools/platform_mappings
    ...
    PiperOrigin-RevId: 301158932
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 19, 2023
@brentleyjones
Copy link
Contributor

@gregestren Are you still working on this?

@gregestren
Copy link
Contributor Author

Yes.

@gregestren gregestren removed the stale Issues or PRs that are stale (no activity for 30 days) label Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

No branches or pull requests

3 participants