To get an absolute path for a file descriptor, we used to call `readlink("/proc/self/fd/{fd}")`. That's perfectly fine and still the best way to do it; the problem is that it is slow. (for example, it is not uncommon that a C compiler calls `putc` millions of times to produce a build output).
This PR implements a file descriptor table where where a mapping from a file descriptor to an absolute path is cached.
A new entry is computed and inserted into the table whenever a path for a file descriptor is needed but it hasn't already been computed.
Because file descriptors may be reused for different files over time, whenever a file descriptor is closed (i.e., the `close` syscall is intercepted) the corresponding entry in the table is invalidated.
Related work items: #1833259
Consider the following scenario:
1. Absent probe on path A/B/C/file.txt
2. Reparse point created on C in path A/B/C, where now A/B/C -> D/E/F and D/E/F/file.txt exists.
3. Present probe on path A/B/C/file.txt (by virtue of 2)
Step 1) populates the reparse point cache with A/B/C/file.txt as not needing resolution (since the path is absent). 2) should invalidate A/B/C, and now A/B/C does need resolution because C is a reparse point. But A/B/C/file.txt does not get invalidated, and therefore when 3) happens, A/B/C/file.txt is not resolved. This manifests in a cache miss when file.txt is a produced output, since we don't compensate for the absence of the file on cache lookup when the unresolved path is stored.
This PR adds:
* A conservative invalidation on reparse point creation (we don't detour the ioctl call, but we see a handle being opened for write and reparse point flags as the immediately previous operation). We invalidate the path in that case to stay on the safe side.
* A tracking mechanism for all descendants of a given path that the reparse point cache knows about. When invalidating a path, all descendants stored in the cache are also invalidated. This accounts for the above scenario, where when C is turned into a reparse point, both A/B/C and A/B/C/file.txt are invalidated
* A lightweight C++ unit test infrastructure (using boost) with unit tests for the new structure.
Related work items: #1824112
This PR updates the version of ErrorProne.NET CoreAnalyzers that now detects an unobserved calls to `CancellationToken.Register`.
It is important to observe and dispose the `CancellationTokenRegistration` instance when they obtained from non-local `CancellationToken`s because otherwise we may ended up with a memory leak when a longer-lived object will hold references to a transient closures created by the callbacks provided to `CancellationToken.Register`.
It seems that the hack I've added to avoid allocations is not needed.
One of the NLog maintainers opened a bug: https://github.com/microsoft/BuildXL/issues/1283 with the explanation of the right solution and this PR uses the proposed solution instead of relying on a custom target I've created previously.
In my little spare time, I found a chance to improve AsyncFixer not to forget my coding skills. Here are changes needed to upgrade to v1.5.1:
- Fixed false negatives, especially for 01:UnnecessaryAsync. That's why I had to remove many redundant async/await keywords. It is always faster to directly return the Task. We rarely need async/await keywords in redundant cases due to some special exception handling. However, we have suppressed "01:UnnecessaryAsync" in only one case at our codebase so far.
- Fixed false positives for other analyzers, especially for 02:BlockingCallInsideAsync. I removed many suppressions, which made me pretty happy.
I am still OOF, but I did not want to postpone these changes. I have also open-sourced AsyncFixer. Feel free to contribute: https://github.com/semihokur/asyncfixer
@<Sergey Tepliakov>, there are several cases where we use async/await keywords with the new using statement syntax. You reported that at some point as far as I remember. I improved AsyncFixer to detect cases like below so that it does not suggest removing async/await
```
async Task foo()
{
using var stream = new MemoryStream();
int streamOperation()
{
return stream.Read(null);
}
Task t = Task.Run(() => streamOperation());
await t;
}
```
@<John Erickson>, could you please point me to the artifact services projects where AsyncFixer is enabled? I can upgrade AsyncFixer in those projects as well.
This PR adds a facility to bxl sdk to use source generators and also it adds one source generator that can generate struct-based record-like behavior as well as two other generators that can:
* Generate ToString implementation similar to records ToString behavior but with the ability to fine tune the final ToString impl.
* Generate equality members.
Related work items: #1805964
Remove TransientFaultHandling completely. Polly has been tested for a while now and this should simplify our dependencies.
The only trace of it left is that ADO packages depend on it.
Related work items: #1807652
This PR adds support for .NET 5.
The only current limitation is that QTest is disabled in .NET 5 because it was failing with some "can't find the right assembly" issue that I'm going to investigate.
Also, the default qualifier stays the same and I need some help testing the changes on the mac and on Linux.
This PR also allow us using C# 9 features like records **for all target frameworks** and the following code compiles just fine:
```csharp
public record X
{
public int Y { get; init; }
}
```
Keep in mind, that you need the most recently released VS in order to see intellisense for this code.
This adds support for LZ4, Zlib and Zstd compression to our RocksDb binaries on Windows. Current compression support is:
- Linux supports lz4 and snappy
- Mac supports lz4 and snappy
- Windows supports lz4, zlib, zstd and snappy
We can build in more if we need. I only added these to the Windows binaries because it's what we'll be using in the datacenter.
Related work items: #1790176
This is the second PR that is required to make the final PR for adding .net5 support simpler.
The PR updates the compiler to a 3.7 version (this version does not support C# 9 yet), and fixes a bunch of nullability warnings that appear after the upgrade to 3.7 and some of them will be visible only when targeting .net 5.
This is another low risk PR.
There is a desire to move to Polly, since TransientFaultHandling is not compatible with regular NuGet when using netstandard.
This adds the capability to put this under a flag to start testing the transition.
Related work items: #1791278
This PR introduces a sandbox implementation based on EndpointSecurity for macOS. It is able to build projects on macOS with full sandboxing and caching (e.g. self-host builds, Apex, etc.). The general architecture is as follows:
- SystemExtension with XPC host as event broker
- BuildXL as `build host` and event manager
- Multiple EndpointSecurity clients with event buckets and dedicated report queues for process observation
Related work items: #1763841, #1763850, #1763852, #1763854
The world moved in the MSBuild SDK and now VB tasks are being built as part of (the same old commit for) Roslyn we use for validating the MSBuild frontend. We were actually missing that case.
Change NodeJs version to appease Component Governance.
Internally, we rely on private repackaged NodeJS, but it has not been updated in a while. So this PR makes everyone consume the public version (from nodejs.org).
Related work items: #1766547, #1766548
Implemented using the auditing API provided by the GNU dynamic linker to observe all dynamically loaded shared libraries:
https://man7.org/linux/man-pages/man7/rtld-audit.7.html
Although the same library can be added to both LD_PRELOAD and LD_AUDIT at the same time, I couldn't do that with `libDetours.so` because it calls `dlsym` which is not available in audit callbacks. That's why the Linux sandbox code is compiled to two (almost identical) libraries, `libDetours.so` and `libBxlAudit.so`; interposing is enabled in the former and disabled in the latter. Both of them use the same `BxlObserver` code to check and report file accesses as soon as they happen.
Related work items: #1741898, #1750766
1. Check the nuget download file using the hashtype given in the hash configured instead of the build's default hashtype.
2. Do not record the hash of the downloaded file in download resolver if the file is not up to date.
Related work items: #1761395, #1761396
This change extracts binding redirects information into a helper method in `BuildXLSdk` and it also adds special comments into the extracted helper and into `config.dsc`. This should help to avoid regressions in the future.
The change also fixes a NRE in tests that was caused by an attempt to reuse not fully initialized `LocalRedisProcessDatabase` instance.
This PR merges the currently experimental version of Roxis against master branch. This is required in order to make progress on testing Roxis in a CBTest stamp.
Note: most of this code is non-productive and hence shouldn't be held to the same standards. We are just making fast progress towards getting a POC working.
Things that do need to be reviewed are all changes to already-productive cache components, simply to ensure the new changes aren't breaking.
Related work items: #1746843
This PR:
- Adds the required assemblies to be able to talk to Azure
- Adds capability to fetch metrics from Azure Monitor, and scale Redis instances
- Creates a rule for autoscaling
- Adds capability to configure the monitor via a Kusto function
- Splits the monitor into 3 dscs: App, Library and Test
The autoscaling algorithm is extremely simple on purpose:
- Fetch metrics from the last week
- Compute the maximum RPS and memory required
- Obtain the cheapest cluster size (i.e. tier and number of shards) that allows us to have an extra 30% memory and 30% expected RPS over the maximum achieved.
The complexity comes from the fact that Azure Redis scaling has plenty of business constraints on when instances can/can't be autoscaled. This means that:
- We can only choose an instance if it is reachable via autoscaling operations from the current instance size.
- When trying to reach a given instance size, we should only do so and not underprovision the stamp. If we underprovision, that could cause a production issue.
This is solved via shortest path graph traversals, which are explained in the code.
Related work items: #1666205, #1698631
Tests performed:
- Backwards compatibility by building BuildXL self-host with the new version and then the old one
- Deployment, by running CloudBuild CI release pipeline: https://dev.azure.com/mseng/Domino/_build/results?buildId=12447913&view=results
- Cache, by deploying manually onto CBTest and validating that we can boot properly. Then undeploy and check it still works.
Aleksandar Milicevic
Oleksii Kononenko
Serge Mera
Sergey Tepliakov
Julian Bayardo
Semih Okur
Rijul Luman
Juan Carlos Guzman Islas
Iman Narasamdya
Nora Huang
Kristijan Simic
Lance Collins
Michael Pysson