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

process: sync commits from Aspect-internal silo #781

Merged
merged 9 commits into from
Nov 30, 2024

Conversation

Copy link

aspect-workflows bot commented Nov 30, 2024

Test

161 test targets passed

Targets
//docs:check_aspect_clean.md [k8-fastbuild]                             86ms
//docs:check_aspect_config.md [k8-fastbuild]                            87ms
//docs:check_aspect_cquery.md [k8-fastbuild]                            122ms
//docs:check_aspect_docs.md [k8-fastbuild]                              274ms
//docs:check_aspect_fetch.md [k8-fastbuild]                             358ms
//docs:check_aspect_info.md [k8-fastbuild]                              69ms
//docs:check_aspect_init.md [k8-fastbuild]                              174ms
//docs:check_aspect_lint.md [k8-fastbuild]                              140ms
//docs:check_aspect_mod.md [k8-fastbuild]                               70ms
//docs:check_aspect_print.md [k8-fastbuild]                             79ms
//docs:check_aspect_shutdown.md [k8-fastbuild]                          125ms
//docs:check_aspect_test.md [k8-fastbuild]                              73ms
//docs:check_aspect_version.md [k8-fastbuild]                           138ms
//gazelle/bzl:bazel_tools_test [k8-fastbuild]                           632ms
//gazelle/bzl:external_test [k8-fastbuild]                              124ms
//gazelle/bzl:multidir_test [k8-fastbuild]                              123ms
//gazelle/bzl:nobuildfiles_test [k8-fastbuild]                          110ms
//gazelle/bzl:private_test [k8-fastbuild]                               166ms
//gazelle/bzl:relative_import_test [k8-fastbuild]                       169ms
//gazelle/bzl:workspace_name_test [k8-fastbuild]                        111ms
//gazelle/common/starlark:starlark_test [k8-fastbuild]                  141ms
//gazelle/js/parser:parser_test [k8-fastbuild]                          250ms
//gazelle/js/typescript:typescript_test [k8-fastbuild]                  477ms
//gazelle/js:declare_module_test [k8-fastbuild]                         806ms
//gazelle/js:declare_module_types_test [k8-fastbuild]                   180ms
//gazelle/js:dynamic_import_test [k8-fastbuild]                         233ms
//gazelle/js:gazelle_disable_conflict_test [k8-fastbuild]               446ms
//gazelle/js:gazelle_disable_test [k8-fastbuild]                        192ms
//gazelle/js:gazelle_exclude_directive_test [k8-fastbuild]              224ms
//gazelle/js:gazelle_generate_build_test [k8-fastbuild]                 197ms
//gazelle/js:gazelle_generation_mode_test [k8-fastbuild]                211ms
//gazelle/js:gazelle_ignore_directive_test [k8-fastbuild]               178ms
//gazelle/js:gazelle_map_kind_directive_test [k8-fastbuild]             216ms
//gazelle/js:groups_add_remove_rules_test [k8-fastbuild]                233ms
//gazelle/js:groups_deps_test [k8-fastbuild]                            180ms
//gazelle/js:groups_simple_files_test [k8-fastbuild]                    227ms
//gazelle/js:ignore_config_files_test [k8-fastbuild]                    236ms
//gazelle/js:js_test [k8-fastbuild]                                     99ms
//gazelle/js:node_native_test [k8-fastbuild]                            429ms
//gazelle/js:npm_link_all_packages_test [k8-fastbuild]                  1s
//gazelle/js:npm_package_deps_lib_test [k8-fastbuild]                   200ms
//gazelle/js:npm_package_deps_test [k8-fastbuild]                       201ms
//gazelle/js:npm_package_deps_tsconfig_test [k8-fastbuild]              1s
//gazelle/js:npm_package_lib_target_name_test [k8-fastbuild]            182ms
//gazelle/js:npm_package_target_enabled_test [k8-fastbuild]             198ms
//gazelle/js:npm_package_target_name_test [k8-fastbuild]                692ms
//gazelle/js:npm_package_target_referenced_test [k8-fastbuild]          1s
//gazelle/js:npm_simple_deps_cjs_test [k8-fastbuild]                    193ms
//gazelle/js:npm_simple_deps_test [k8-fastbuild]                        548ms
//gazelle/js:parse_errors_test [k8-fastbuild]                           694ms
//gazelle/js:pnpm_project_refs_lock5_test [k8-fastbuild]                3s
//gazelle/js:pnpm_project_refs_lock6_test [k8-fastbuild]                219ms
//gazelle/js:pnpm_workspace_rerooted_test [k8-fastbuild]                208ms
//gazelle/js:pnpm_workspace_test [k8-fastbuild]                         234ms
//gazelle/js:rules_conflicting_name_mapped_kind_test [k8-fastbuild]     169ms
//gazelle/js:rules_conflicting_name_test [k8-fastbuild]                 130ms
//gazelle/js:rules_ordering_test [k8-fastbuild]                         217ms
//gazelle/js:simple_dts_only_dep_test [k8-fastbuild]                    235ms
//gazelle/js:simple_file_test [k8-fastbuild]                            270ms
//gazelle/js:simple_globs_keep_test [k8-fastbuild]                      211ms
//gazelle/js:simple_globs_test [k8-fastbuild]                           213ms
//gazelle/js:simple_import_disabled_test [k8-fastbuild]                 177ms
//gazelle/js:simple_import_generated_test [k8-fastbuild]                222ms
//gazelle/js:simple_imports_cjs_test [k8-fastbuild]                     382ms
//gazelle/js:simple_rule_naming_directives_test [k8-fastbuild]          210ms
//gazelle/js:tests_new_test [k8-fastbuild]                              189ms
//gazelle/js:tests_nolib_test [k8-fastbuild]                            209ms
//gazelle/js:tests_simple_test [k8-fastbuild]                           255ms
//gazelle/js:tests_subproject_test [k8-fastbuild]                       219ms
//gazelle/js:ts_proto_library_test [k8-fastbuild]                       231ms
//gazelle/js:tsconfig_attrs_inherited_test [k8-fastbuild]               190ms
//gazelle/js:tsconfig_baseurl_test [k8-fastbuild]                       202ms
//gazelle/js:tsconfig_invalid_test [k8-fastbuild]                       446ms
//gazelle/js:tsconfig_lax_json_test [k8-fastbuild]                      439ms
//gazelle/js:tsconfig_manual_test [k8-fastbuild]                        134ms
//gazelle/js:tsconfig_nomore_configs_test [k8-fastbuild]                203ms
//gazelle/js:tsconfig_outdir_test [k8-fastbuild]                        553ms
//gazelle/js:tsconfig_paths_test [k8-fastbuild]                         208ms
//gazelle/js:tsconfig_pnpm_ref_rerooted_test [k8-fastbuild]             973ms
//gazelle/js:tsconfig_rootdir_test [k8-fastbuild]                       175ms
//gazelle/js:tsconfig_rootdirs_test [k8-fastbuild]                      464ms
//gazelle/js:validate_import_statements_off_test [k8-fastbuild]         534ms
//gazelle/js:visibility_test [k8-fastbuild]                             268ms
//gazelle/kotlin:bin_test [k8-fastbuild]                                314ms
//gazelle/kotlin:gazelle_disable_conflict_test [k8-fastbuild]           193ms
//gazelle/kotlin:gazelle_disable_test [k8-fastbuild]                    173ms
//gazelle/kotlin:native_deps_test [k8-fastbuild]                        277ms
//gazelle/kotlin:rules_conflicting_name_mapped_kind_test [k8-fastbuild] 454ms
//gazelle/kotlin:simple_file_test [k8-fastbuild]                        228ms
//gazelle/kotlin:unknown_imports_test [k8-fastbuild]                    124ms
//integration_tests/aspect:help_test [k8-fastbuild]                     47s
//integration_tests/aspect:info_test [k8-fastbuild]                     25s
//integration_tests/aspect:reenter_test [k8-fastbuild]                  45s
//integration_tests/aspect:version_test [k8-fastbuild]                  43s
//pkg/aspect/cquery:cquery_test [k8-fastbuild]                          172ms
//pkg/aspect/lint:lint_test [k8-fastbuild]                              120ms
//pkg/aspect/root/config:config_test [k8-fastbuild]                     86ms
//pkg/aspect/run:run_test [k8-fastbuild]                                77ms
//pkg/aspect/test:test_test [k8-fastbuild]                              78ms
//pkg/bazel:bazel_test [k8-fastbuild]                                   962ms
+ 61 other targets

Total test execution time was 8m 40s. 47 tests (22.6%) were fully cached saving 13s.


Buildifier      Format

jbedard and others added 9 commits November 29, 2024 16:36
- Covered by existing test cases

GitOrigin-RevId: b77e25c5dce88015b2d7a80d78ba14203590153b
---

### Changes are visible to end-users: no

### Test plan

- Manual testing; please provide instructions so we can reproduce:
  bazel test //workflows/bessie/test:e2e --test_output=streamed --test_filter="correlates failing actions with target completions"

GitOrigin-RevId: fd87dbf9b0cdcc7a6620f47798ff6ce1bc410e72
`sync.Map` seems optimal for this exact use case where caching has no
evictions and only grows.

> The Map type is optimized for two common use cases: (1) when the entry
for a given key is only ever written once but read many times, as in
caches that only grow, or (2) when multiple goroutines read, write, and
overwrite entries for disjoint sets of keys. In these two cases, use of
a Map may significantly reduce lock contention compared to a Go map
paired with a separate [Mutex](https://pkg.go.dev/sync#Mutex) or
[RWMutex](https://pkg.go.dev/sync#RWMutex).

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 79fb097ab5bee25dcd6c7b9e1f760dc8dcc92753
…#7296)

This avoids putting the entire lockfile into memory all at once just to
iterate over it once and throw it away.

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Memory improvements to js `aspect configure`

### Test plan

- Covered by existing test cases

GitOrigin-RevId: ac5ef37ffcdba9db4108632d62fdfc9f4de8b981
Caching the parsing + compiling of jq queries.

---

- Covered by existing test cases

GitOrigin-RevId: c49200173440e195c41fc58894ddcfa9539127e9
…7326)

These simple string operations are sometimes noticeable in memory
profiles, at least in the utils invoked multiple times per file in repos
of 50k+ files.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: ff0100238cbe0b77d54c629456ffe2678ccf8342
…7335)

Simply invoking `node.Type()` 3x for every root node for 45k .ts files
was noticeable when memory profiling.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 89e73b1040c33a451c2cb3778c236b454b102a39
The patch changes are primarily due to
bazel-contrib/bazel-gazelle@ed19735

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: bed62a066c8ab87b77cabee01eadd2ffaf718d9b
…(#7357)

EXPERIMENTAL:

An initial internal API for caching data to disk between aspect
configure runs. This must be manually enabled with the
`ASPECT_CONFIGURE_CACHE` env var which points to the cache file.

Initially this is being used by the js and starzelle languages to cache
source code queries, primarily tree-sitter parse+query but also things
like `jq` or regex queries are covered in the starzelle case.

On a large customer repo this reduces the total CPU time from ~55s to
12s, although the wall time the user sees is more around 16s to 9s (=
40% faster, roughly aligning with my profiling showing).

Examples, pre-cache:
```
56.08s user 14.16s system 437% cpu 16.056 total
56.73s user 16.21s system 410% cpu 17.765 total
```
Post-cache:
```
11.69s user 15.04s system 295% cpu 9.049 total
12.12s user 16.74s system 305% cpu 9.459 total
12.05s user 8.44s system 202% cpu 10.133 total
```

---

### Changes are visible to end-users: yes

- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes

Add experimental caching of source code analysis for `aspect configure`
via `ASPECT_CONFIGURE_CACHE` environment variable pointing to a cache
file path, normally in a temp or ignored file location.

### Test plan

- Covered by existing test cases
- Manual testing; please provide instructions so we can reproduce: run
`aspect configure` on a large repo with the env var

GitOrigin-RevId: 399d18a70277a58ed56288ceeb6e2f1ff025a9e6
@jbedard jbedard force-pushed the 788EA150BB0FAAB4A3FF81F26DCBE189 branch from bc6caa5 to 31aef06 Compare November 30, 2024 00:36
@jbedard jbedard merged commit 848653c into main Nov 30, 2024
10 checks passed
@jbedard jbedard deleted the 788EA150BB0FAAB4A3FF81F26DCBE189 branch November 30, 2024 00:58
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.

3 participants