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

feat(resolve): optimize applying overrides to be efficient #1687

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

tyler-french
Copy link
Contributor

@tyler-french tyler-french commented Dec 13, 2023

What type of PR is this?

Feature

What package or component does this PR mostly affect?

resolve

Enhancements in Resolve Directive Handling

This PR addresses the inefficiency of processing # gazelle:resolve directives. Previously, the system iterated over a slice for each import to find matching directives, which is very inefficient. This PR introduces two major improvements:

  1. Transition from Slice to Map: We've shifted from using a slice to a map for storing override configurations. This change reduces lookup time complexity from O(n) to O(1) (or O(length of chain of enclosing dirs containing resolves)).

  2. Parent-Child Config Structure: To maintain directory-specific configurations without the overhead of copying maps (which is very inefficient), we've implemented a parent-child relationship in configurations. This structure allows each directory to have its unique overrides while inheriting unmodified settings from its parent. We can't use a single shared map, because this could mistakenly apply directives from one directory to another. regexp overrides are always copied and evaluated inline.

Note on GC: to avoid a really complex object tree, the parent/child relationship skips all ancestors without # gazelle:resolve directives, greatly reducing the complexity.

Note of Regexp: although we need to iterate through these due to the matching logic, we could at least have a map: lang -> []overrides to improve the lookup some.

Data from Uber

We have ~millions of directories to traverse, and approximately 8000 resolve directives.

Profile before changes:
Screenshot 2023-12-13 at 4 20 56 PM

Profile using a full map copy (on small directory), showing the inefficiencies of copying a map each time:
Screenshot 2023-12-13 at 4 20 26 PM

Which issues(s) does this PR fix?

Fixes #1688

@tyler-french tyler-french changed the title optimize resolve feat(resolve): optimize applying overrides to be efficient Dec 13, 2023
@tyler-french tyler-french force-pushed the optimize-resolve branch 2 times, most recently from f7c4dcf to 8ea722d Compare December 13, 2023 22:47
@tyler-french tyler-french marked this pull request as ready for review December 13, 2023 22:51
Copy link
Collaborator

@fmeum fmeum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff!

resolve/resolve_test.go Show resolved Hide resolved
@tyler-french
Copy link
Contributor Author

@fmeum Let me do some additional testing to make sure no unexpected behaviors are changed.

@tyler-french
Copy link
Contributor Author

@fmeum Let me do some additional testing to make sure no unexpected behaviors are changed.

Ok, I ran a bunch of tests and they all seemed to behave as expected. Let me check to see if @linzhp has any hesitations here

@tyler-french tyler-french force-pushed the optimize-resolve branch 2 times, most recently from 21a0d3a to 15d84af Compare December 15, 2023 04:10
@tyler-french tyler-french enabled auto-merge (squash) December 15, 2023 16:03
@tyler-french tyler-french merged commit 073699e into bazelbuild:master Dec 15, 2023
9 of 10 checks passed
renovate bot added a commit to kreempuff/rules_unreal_engine that referenced this pull request Dec 21, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) |
http_archive | minor | `v0.34.0` -> `v0.35.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>bazelbuild/bazel-gazelle (bazel_gazelle)</summary>

###
[`v0.35.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.35.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.34.0...v0.35.0)

#### What's Changed

- Don't run on centos7 in BCR presubmit and add maintainers by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1663
- update readme for v0.34.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1662
- nit: doc `gazelle_generation_test` use of `arguments.txt` by
[@&#8203;vpanta](https://togithub.com/vpanta) in
[bazelbuild/bazel-gazelle#1660
- feat(bzlmod): allow patches in `archive_override`s by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1665
- fix(bzlmod): fail on unused overrides by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1669
- Annotate more globs with `allow_empty = True` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1674
- Generate BUILD files for grpc compiler by
[@&#8203;mering](https://togithub.com/mering) in
[bazelbuild/bazel-gazelle#1672
- Disable lockfile check for BCR test module by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1673
- Make isolated extensions usable for Go tools by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1678
- bzlmod: support go.mod replace directives with a version qualifier on
the left by [@&#8203;andyscott](https://togithub.com/andyscott) in
[bazelbuild/bazel-gazelle#1679
- fix CI by running both WORKSPACE and Bzlmod on select packages by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1690
- disable lockfile by default by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1692
- feat(resolve): optimize applying overrides to be efficient by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1687
- feat(fix-update): allow user to profile commands with `pprof` by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1685
- Fix partial execution with mapped kinds by
[@&#8203;HALtheWise](https://togithub.com/HALtheWise) in
[bazelbuild/bazel-gazelle#1680
- prepare release 0.35.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1693

#### New Contributors

- [@&#8203;mering](https://togithub.com/mering) made their first
contribution in
[bazelbuild/bazel-gazelle#1672

**Full Changelog**:
bazelbuild/bazel-gazelle@v0.34.0...v0.35.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/kreempuff/rules_unreal_engine).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMDMuMSIsInVwZGF0ZWRJblZlciI6IjM3LjEwMy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
cgrindel-self-hosted-renovate bot added a commit to cgrindel/bazel-starlib that referenced this pull request Dec 21, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) |
http_archive | minor | `v0.34.0` -> `v0.35.0` |

---

### Release Notes

<details>
<summary>bazelbuild/bazel-gazelle (bazel_gazelle)</summary>

###
[`v0.35.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.35.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.34.0...v0.35.0)

#### What's Changed

- Don't run on centos7 in BCR presubmit and add maintainers by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1663
- update readme for v0.34.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1662
- nit: doc `gazelle_generation_test` use of `arguments.txt` by
[@&#8203;vpanta](https://togithub.com/vpanta) in
[bazelbuild/bazel-gazelle#1660
- feat(bzlmod): allow patches in `archive_override`s by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1665
- fix(bzlmod): fail on unused overrides by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1669
- Annotate more globs with `allow_empty = True` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1674
- Generate BUILD files for grpc compiler by
[@&#8203;mering](https://togithub.com/mering) in
[bazelbuild/bazel-gazelle#1672
- Disable lockfile check for BCR test module by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1673
- Make isolated extensions usable for Go tools by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1678
- bzlmod: support go.mod replace directives with a version qualifier on
the left by [@&#8203;andyscott](https://togithub.com/andyscott) in
[bazelbuild/bazel-gazelle#1679
- fix CI by running both WORKSPACE and Bzlmod on select packages by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1690
- disable lockfile by default by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1692
- feat(resolve): optimize applying overrides to be efficient by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1687
- feat(fix-update): allow user to profile commands with `pprof` by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1685
- Fix partial execution with mapped kinds by
[@&#8203;HALtheWise](https://togithub.com/HALtheWise) in
[bazelbuild/bazel-gazelle#1680
- prepare release 0.35.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1693

#### New Contributors

- [@&#8203;mering](https://togithub.com/mering) made their first
contribution in
[bazelbuild/bazel-gazelle#1672

**Full Changelog**:
bazelbuild/bazel-gazelle@v0.34.0...v0.35.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
cgrindel-self-hosted-renovate bot added a commit to cgrindel/rules_swift_package_manager that referenced this pull request Dec 22, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) |
http_archive | minor | `v0.34.0` -> `v0.35.0` |

---

### Release Notes

<details>
<summary>bazelbuild/bazel-gazelle (bazel_gazelle)</summary>

###
[`v0.35.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.35.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.34.0...v0.35.0)

#### What's Changed

- Don't run on centos7 in BCR presubmit and add maintainers by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1663
- update readme for v0.34.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1662
- nit: doc `gazelle_generation_test` use of `arguments.txt` by
[@&#8203;vpanta](https://togithub.com/vpanta) in
[bazelbuild/bazel-gazelle#1660
- feat(bzlmod): allow patches in `archive_override`s by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1665
- fix(bzlmod): fail on unused overrides by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1669
- Annotate more globs with `allow_empty = True` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1674
- Generate BUILD files for grpc compiler by
[@&#8203;mering](https://togithub.com/mering) in
[bazelbuild/bazel-gazelle#1672
- Disable lockfile check for BCR test module by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1673
- Make isolated extensions usable for Go tools by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1678
- bzlmod: support go.mod replace directives with a version qualifier on
the left by [@&#8203;andyscott](https://togithub.com/andyscott) in
[bazelbuild/bazel-gazelle#1679
- fix CI by running both WORKSPACE and Bzlmod on select packages by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1690
- disable lockfile by default by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1692
- feat(resolve): optimize applying overrides to be efficient by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1687
- feat(fix-update): allow user to profile commands with `pprof` by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1685
- Fix partial execution with mapped kinds by
[@&#8203;HALtheWise](https://togithub.com/HALtheWise) in
[bazelbuild/bazel-gazelle#1680
- prepare release 0.35.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1693

#### New Contributors

- [@&#8203;mering](https://togithub.com/mering) made their first
contribution in
[bazelbuild/bazel-gazelle#1672

**Full Changelog**:
bazelbuild/bazel-gazelle@v0.34.0...v0.35.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
@tyler-french tyler-french deleted the optimize-resolve branch December 23, 2023 18:41
pcj added a commit to stackb/rules_proto that referenced this pull request Jan 3, 2024
alexeagle pushed a commit to aspect-build/rules_py that referenced this pull request Feb 2, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [bazel_gazelle](https://togithub.com/bazelbuild/bazel-gazelle) |
http_archive | minor | `v0.34.0` -> `v0.35.0` |

---

### Release Notes

<details>
<summary>bazelbuild/bazel-gazelle (bazel_gazelle)</summary>

###
[`v0.35.0`](https://togithub.com/bazelbuild/bazel-gazelle/releases/tag/v0.35.0)

[Compare
Source](https://togithub.com/bazelbuild/bazel-gazelle/compare/v0.34.0...v0.35.0)

#### What's Changed

- Don't run on centos7 in BCR presubmit and add maintainers by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1663
- update readme for v0.34.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1662
- nit: doc `gazelle_generation_test` use of `arguments.txt` by
[@&#8203;vpanta](https://togithub.com/vpanta) in
[bazelbuild/bazel-gazelle#1660
- feat(bzlmod): allow patches in `archive_override`s by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1665
- fix(bzlmod): fail on unused overrides by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1669
- Annotate more globs with `allow_empty = True` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1674
- Generate BUILD files for grpc compiler by
[@&#8203;mering](https://togithub.com/mering) in
[bazelbuild/bazel-gazelle#1672
- Disable lockfile check for BCR test module by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1673
- Make isolated extensions usable for Go tools by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/bazel-gazelle#1678
- bzlmod: support go.mod replace directives with a version qualifier on
the left by [@&#8203;andyscott](https://togithub.com/andyscott) in
[bazelbuild/bazel-gazelle#1679
- fix CI by running both WORKSPACE and Bzlmod on select packages by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1690
- disable lockfile by default by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1692
- feat(resolve): optimize applying overrides to be efficient by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1687
- feat(fix-update): allow user to profile commands with `pprof` by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1685
- Fix partial execution with mapped kinds by
[@&#8203;HALtheWise](https://togithub.com/HALtheWise) in
[bazelbuild/bazel-gazelle#1680
- prepare release 0.35.0 by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/bazel-gazelle#1693

#### New Contributors

- [@&#8203;mering](https://togithub.com/mering) made their first
contribution in
[bazelbuild/bazel-gazelle#1672

**Full Changelog**:
bazelbuild/bazel-gazelle@v0.34.0...v0.35.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/aspect-build/rules_py).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
pcj added a commit to stackb/rules_proto that referenced this pull request Feb 13, 2024
* Convert to yaml config
* Vendor timestamp.proto (for ts_proto)
* Switch to proto_gazelle rule
* Fix missing NonEmptyAttrs
* Reset baseline generated rules using proto_gazelle
* Upgrade rules_go, gazelle, go_deps
* Upgrade gazelle to v0.35.0
* Migrate resolver to accomodate bazelbuild/bazel-gazelle#1687
* checkpoint migrate to protobuf-javascript
* Use java 17, upgrade rules_scala, custom toolchain
* sync scala version
* Upgrade rules_python and add to WORKSPACE boilerplate files
* Add :protobuf_python to example deps
* ensure strings sorted for import labels
* Upgrade rules_go to v0.45.1
* exclude grpc_test.go from default go_test rule
* remove obsolete patch
* Add "make golden_test" target
* ensure reassign memory value
* Update build status badge
* preserve last-wins semantics of prior stackb/rules_proto behavior
* Update release.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

# gazelle:resolve directives are very inefficient
3 participants