Cc shared library windows support #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This better matches the text in defines,local_defines on cc_binary. PiperOrigin-RevId: 412075161
The current genrule setup is missing `genrule-setup.sh` file, making each `genrule` target fail in tests. Use `MockGenruleSupport` which sets up all necessary files instead of creating only the `BUILD` file. PiperOrigin-RevId: 412086839
Fetching a repository is a long-running operation that can easily be interrupted. If it is and the marker file exists on disk, a new evaluation of the RepositoryDelegatorFunction may treat this repository as valid even though it is in an inconsistent state. Clearing the marker file before initiating the fetch and only recreating it after the fetch is complete prevents this scenario. Fixes bazelbuild#8993. Closes bazelbuild#14302. PiperOrigin-RevId: 412101756
…code reuse and request batching. While refactoring `AspectFunction`, I've noticed that Skyframe requests may be suboptimally batched when different keys throw different exception types because the author probably felt that batching them introduced too much complexity. Another pattern I'm seeing is the use of helper functions which may expect only a certain exception type and/or arity of `ValueOrExceptionN`, leading to suboptimal batching to fit the method's requirements. An example is `ConfiguredTargetFunction#computeDependencies`, where aspects are not requested along with configured targets - requesting them together would make calling into the helper function `ConfiguredTargetFunction#resolveConfiguredTargetDependencies` difficult. The new methods in this change already have some use cases and will additionally allow me to batch configured target and aspect requests as mentioned above without introducing code duplication. PiperOrigin-RevId: 412120270
PiperOrigin-RevId: 412242583
When setting `--experimental_remote_merkle_tree_cache=true` as default and running the test suite, some errors became visible. This change fixes those errors. Closes bazelbuild#14300. PiperOrigin-RevId: 412274673
Downstream tests https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2258 show no new failures. PiperOrigin-RevId: 412275137
PiperOrigin-RevId: 412278388
Moved content from cc_semantics.bzl to semantics.bzl. PiperOrigin-RevId: 412280712
…lify ProtoCompileActionBuilder. `$(OUT)` placeholder is replaced with `%s` so it can be directly used in addFormatted. This simplifies construction of proto compile action and makes it possible for Starlark version to have the same performance. github shows no uses of the placeholder: https://github.com/search?q=%22%24%28PLUGIN_OUT%29%22&type=Repositories PiperOrigin-RevId: 412286743
This makes it possible to pair it with command_line attribute which could contain dependent data,
for example:
```
proto_lang_toolchain(
name = "j2objc_proto_toolchain",
blacklisted_protos = [],
command_line = "--PLUGIN_j2objc_out=file_dir_mapping,generate_class_mappings:$(OUT)",
+ plugin_format_flag = "--plugin=protoc-gen-PLUGIN_j2objc=%s",
plugin = "//third_party/java/j2objc:proto_plugin",
runtime = "//third_party/java/j2objc:proto_runtime",
visibility = ["//visibility:public"],
)
```
It also retains reference to the flag on proto_lang_toolchain rule and so doesn't cause a memory regression when proto_lang_libraries are Starlarkyfied.
PiperOrigin-RevId: 412287195
CcCompilationOutputs object being passed to the link command, instead of filtered one(only with objects, pic_objects and lto_compilation_context). PiperOrigin-RevId: 412409684
….build.lib.dynamic.DynamicSpawnStrategy@6fd1927c` or similar in logging output. PiperOrigin-RevId: 412424315
PiperOrigin-RevId: 412444511
This CL adds `--aspects_parameters` option to pass values for command-line aspects specified via `--aspects` option. The new option `--aspects_parameters` is only effective with `--experimental_allow_top_level_aspects_parameters` flag. PiperOrigin-RevId: 412476585
Testing the outer method will make it possible to remove inner method later. PiperOrigin-RevId: 412825029
PiperOrigin-RevId: 412825461
PiperOrigin-RevId: 412840929
One of Buildbarn's users is attempting to build it on a Mac M1 system that does not have Rosetta installed: buildbarn/bb-remote-execution#89 This currently fails with the following error message: ERROR: <storage>/external/com_google_protobuf/BUILD:130:11: Compiling src/google/protobuf/extension_set.cc failed: I/O exception during sandboxed execution: com.google.devtools.build.lib.shell.ExecFailedException: java.io.IOException: Cannot run program "<tmp>/install/71ed47cad951a20fff87381f54639763/xcode-locator": error=86, Bad CPU type in executable Let's address this by shipping a copy of xcode-locator that is built both for ARM64 and x86-64. Closes bazelbuild#14168. PiperOrigin-RevId: 412864310
This is needed for very few rules, primarily only in cpp, java and python/proto. Keeping it in cc_internal for now, to avoid cluttering the ctx.actions.declare_file() PiperOrigin-RevId: 412867088
…st runner PiperOrigin-RevId: 412870758
This will change the contents of JavaInfo as seen by Starlark code but is necessary for single_jar to extract transitive deps for java_binary inputs. This was earlier extracted using compilation_info (which we are removing) PiperOrigin-RevId: 412872979
Check-in a functional cc_test implementation. A few fixes to cc_binary impl: * refactored to make binary_impl accessible to other rules. * split out attrs so that test can reuse * Fix a couple calls to `create_library_to_link` using all named arguments (rather than a mix of named and positional). * Updated custom_malloc handling. * Inspect an _is_test attribute, rather than looking at the label name. Not all tests are named "foo_test", there are lots of other common patterns (e.g. "foo_unittest"). PiperOrigin-RevId: 412889678
When running on Windows, Bazel keeps a handle open for `windows_jni.dll` which makes it impossible to delete all files and directories from the test root after each test case. The current strategy is to ignore failures in the cleanup on Windows which puts tests at risk of running from a polluted directory since `Path::deleteTreesBelow` stops deleting files at first failure. Apply a best-effort deletion strategy on Windows instead, allowing to leave the `windows_jni.dll` behind, but deleting all other files. PiperOrigin-RevId: 412897053
PiperOrigin-RevId: 412898770
PiperOrigin-RevId: 412900415
PiperOrigin-RevId: 412915691
…l request in `AspectFunction`. PiperOrigin-RevId: 412955595
…mentFactory This is a little cleaner as now BuildConfigurationFunction is simpler and it is clear what code is part of fragment and creation (moved to FragmentFactory). PiperOrigin-RevId: 412963297
Adds test coverage for an aspect on a late-bound alias whose `ConfiguredTarget` turns out not to be an alias. PiperOrigin-RevId: 412965476
PiperOrigin-RevId: 417821085
It is somewhat confusing that a subclass can override `newNodeEntry()`, rendering the value passed to `keepEdges` irrelevant. PiperOrigin-RevId: 417829239
PiperOrigin-RevId: 417836149
…nd `EvaluationContext#getCopyWithKeepGoing` methods. For the former, use inheritance. For the latter, simply implement it manually. The `return this` optimization is unnecessary; this code is called exactly once per SkyQuery `blaze query` invocation, so we're saving a few ns of wall time at most. PiperOrigin-RevId: 417844698
…arning that we are fully qualifying class names. PiperOrigin-RevId: 417847683
…ext` in the Blaze-on-Skyframe codebase. A future commit will make use of this entry-point. PiperOrigin-RevId: 417850794
PiperOrigin-RevId: 417853642
…or noticing memory pressure and (ii) a use of that mechanism to achieve the actual goal of `RetainedHeapLimiter`. This accomplishes two things: * Strict decrease in code complexity. * By using a custom `BlazeModule`, we can get rid of the special-casing in `BlazeCommandDispatcher` and `BlazeRuntime`. * The new `RetainedHeapLimiter` is simpler since the code for noticing memory pressure has been factored out. Check out the unit tests :) * Opens the door for new functionality triggered by memory pressure. I will be adding one myself in the very near future. And I can imagine other pieces of functionality we can add too. PiperOrigin-RevId: 417856795
My previous commit used the wrong target name for AutoValue. PiperOrigin-RevId: 417871966
…utable set. PiperOrigin-RevId: 417874934
…e working with --incompatible_remote_results_ignore_disk. Fixes bazelbuild#14463. Closes bazelbuild#14468. PiperOrigin-RevId: 417984062
…ude action registration. This makes it possible to use Starlarkified create_proto_compile_action call. PiperOrigin-RevId: 417992348
…beRegister. PiperOrigin-RevId: 417992740
This should save some memory allocations. PiperOrigin-RevId: 417993065
This should save some memory allocations. PiperOrigin-RevId: 417993342
PiperOrigin-RevId: 418027309
PiperOrigin-RevId: 418466960
…rame evaluation, it drops all the temporary `SkyKeyComputeState` instances. This fully addresses the performance problem that I described in the description of bazelbuild@ed279ab (I summarized this problem here in a comment in `SkyframeHighWaterMarkLimiter.java`), thus allowing us to make full use of `SkyKeyComputeState` in heavy hitters in the Blaze-on-Skyframe codebase and get strict CPU and wall time wins in all situations. Look forward to a future CL that does this for `ConfiguredTargetFunction` and `AspectFunction` :) Alternatives considered ----- After reimplementing `ConfiguredTargetFunction` and `AspectFunction` to use `SkyKeyComputeState` I tried several different approaches for addressing this problem. None of them worked. 0. Do nothing. On various benchmarks where Blaze was not memory constrained, I was getting 3-5% reductions in both CPU time and wall time. Good! This was always the initial motivation of this project. On various benchmarks where Blaze **was** memory constrained, I was getting 7-10% increases in both CPU time and wall time. Bad! This was unacceptable. 1. Have `SkyKeyComputeStateManager` use a bounded cache `SkyKeyComputeState` (including having different per-`SkyFunctionName` bounds). I spent a while on this. For a specific target, I was able to come up with precise bounds on the cache sizes that let me mitigate the regression when Blaze was memory constrained. There was still a regression though, and so this approach is definitely inferior to what I ended up doing in this CL. There's also the massive issue of: How would we set the cache bounds to achieve good results in all situations? This is clearly impossible: Each combo of `(blaze invocation, blaze Xmx)` would want different choices for the cache bounds. Even if I were able to come up with some cache bounds that are good for common situations internally at Google, there's no way they would be good for arbitrary Bazel usage elsewhere in the world. And even ignoring that, any static cache bounds would definitely grow stale as Blaze's implementation changes and the code it's being asked to build changes. Therefore, this approach was unacceptable. 2. Have `SkyKeyComputeStateManager` use a soft cache. This sounds good in theory, since `SoftReference`s are collected only when the JVM thinks its under memory pressure. But there are two problems * There's a GC performance penalty to using `SoftReference`s since the JVM has to scan all of them to check reachability. * Some usages of Blaze inside Google tweak JVM settings controlling `SoftReference` collection behavior. All of those usages would have to be re-tweaked for this new thing. And it may not be possible to reconcile the existing thing with the new thing. Also, some usages of Bazel might be similarly tweaking JVM settings. I didn't think it'd be good to tell Bazel users "Hi, Bazel now crucicially uses SoftReference. You'll have to tweak JVM settings yourself to get decent performance". In my benchmarks, this approach was a lot worse than (0) when Blaze was memory constrained (something like a net reduction of only 1-2% CPU and wall time), and still yielded an overall regression when Blaze was memory constrained. So this is still unacceptable. 3. Same as (2) but with `WeakReference`s. The JVM clears `WeakReference`s even more aggressively than `SoftReference`s. So, while the GC performance penalty noted above doesn't apply, our usage will definitely suffer since `SkyKeyComputeStateManager` itself will be logically thrashing. My benchmarks confirmed this intuition. The results weren't good enough. 4. Combo of (1) & (2)/(3). That is, when entries get evicted from the soft/weak cache(s), put them in strong bounded cache(s), respecting the LRU policy. I spent a while on this and was able to get decent benchmark results, but nothing close to what I got with the approach I ended up doing in this CL. 5. Same as (4) but with "the approach in this CL" rather than (2)/(3). I didn't try this because I was very pleased with the positive results of this CL. And I was still concerned about the downsides of (1). Done well, I could imagine this being a further improvement in the future though. PiperOrigin-RevId: 418537075
PiperOrigin-RevId: 418612404
Replace `SkyKeyComputeState SkyFunction.createNewSkyKeyComputeState()` & `SkyFunction.compute(SkyKey, SkyKeyComputeState, Environment)` with `T Environment#getState(Supplier<T>)` This accomplishes three things: (1) Improves readability of Blaze-on-Skyframe code by making everything more concise and making the `SkyFunction#compute` method less cluttered (now, due to this CL here and bazelbuild@927b625, the `SkyFunction` interface has only one method that needs to be implemented). Notably, see the final bullet point of "Implementation notes" in the description of bazelbuild@ed279ab. Yes, this CL here is the solution to the issue there :) (2) The new `SkyFunction.Environment#getState` is actually strictly more powerful than the old `SkyFunction#createNewSkyKeyComputeState` because `SkyFunction.Environment` is associated with a specific `SkyKey` while `SkyFunction` is, of course, not. This additional power may be useful for the memory performance TODO in `ActionExecutionFunction` (due to "shared actions"). (3) Improves readability of the Skyframe engine implementation by making the `SkyKeyComputeStateManager` abstraction unnecessary. I originally thought that abstraction would be useful for the high water mark memory concern (e.g. by maintaining per-SkyFunctionName caches), but I fixed that concern already via bazelbuild@343ba43. See the description of that CL for details. PiperOrigin-RevId: 418639155
…dent. Also updated tests in OutputArtifactConflictTest to not use cc_* targets. This allows us to decouple the dependency on these cc_* rules and in general more control over the tests. RELNOTES: None PiperOrigin-RevId: 418764662
…kerTestBase into FileSystemValueCheckerTest. FileSystemValueCheckerTest now has TestParameterInjector to do the parameterizing work. Also there is no need to keep FilesystemValueCheckerTestBase since FileSystemValueCheckerTest is the only class extending from it. PiperOrigin-RevId: 418790548
…luator`. PiperOrigin-RevId: 418826862
getPool was reusing the same thread pool across tests, instead create new pools for each test. advanceCurrentTimeSeconds' name gives the illusion it actually moves the clock, as is customary for mock clocks these days, however it actually slept, which isn't ideal in tests either. The rest is unused. PiperOrigin-RevId: 418832178
This makes it clearer that this flag affects the BES and not, e.g., remote cache or execution. `--bes_instance_name` was chosen to match `--remote_instance_name` as closely as possible. `--project_id` always seemed very Google/GCP centric, and now that there are multiple other BES implementations like BuildBuddy or EngFlow Build and Test UI, it makes sense to change the terminology to follow the conventions from the REAPI. We will keep `--project_id` as an alias for the foreseable future. Closes bazelbuild#14455. PiperOrigin-RevId: 418972581
Avoid creating a boxed `Long`, and add the typical fast-path for reference equality. PiperOrigin-RevId: 418983086
This should fix bazelbuild#14343. PiperOrigin-RevId: 419401678
Deduplicate warnings in terminal. This was working in earlier bazel versions both for read and write, but become broken when write was moved to RemoteExecutionService.java by the "Remote: Async upload" set of commits, completed by commit 581c81a. Use same phrase "Remote Cache" for both read and write, for deduplication to work better. Avoid printing short warnings on multiple lines for reads, as it already was for writes. Closes bazelbuild#14442. PiperOrigin-RevId: 419442535
oquenchil
pushed a commit
that referenced
this pull request
Dec 19, 2023
… order.
1. `TestAttempt` events would wait for the `TargetCompleteEvent` to be posted before being posted.
2. There was an implicit requirement for the `TestAttempt` events to be posted in a specific order.
3. This didn't break in the noskymeld case because we fulfilled this ordering by using the order of performing the attempts themselves. The sequence would look like:
+ post `TargetCompleteEvent`
-> perform attempt #1
-> post `TestAttempt` #1
-> perform attempt bazelbuild#2
-> post `TestAttempt` bazelbuild#2
4. With skymeld, however, it could happen like this:
+ defer `TargetCompleteEvent` to wait for `CoverageActionFinishedEvent`
+ perform attempt #1 -> defer posting `TestAttempt` #1 & wait for `TargetCompleteEvent`
+ perform attempt bazelbuild#2 -> defer posting `TestAttempt` bazelbuild#2 & wait for `TargetCompleteEvent`
+ `CoverageActionFinishedEvent` -> release & post `TargetCompleteEvent`
+ `TargetCompleteEvent` -> release & post `TestAttempt` bazelbuild#2
+ `TargetCompleteEvent` -> release & post `TestAttempt` #1
Due to (2), the undefined ordering in (4) would cause an issue.
This CL fixes that by ensuring a FIFO ordering of the deferred events.
PiperOrigin-RevId: 572165337
Change-Id: Iac4d023d946865b8b81f15b119417192dc4b5c53
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.