Summary:
Looking at Instruments, when issuing tons of glob queries to EdenFS, EdenFS
appears to be spending a very large amount of time adding tasks to the
UnboundedTaskExecutor. Since globs are expected to be fast, we can afford to
execute them inline, reducing this overhead and speeding up glob queries.
Reviewed By: chadaustin
Differential Revision: D31289485
fbshipit-source-id: 428fff9f5fea65073b2a061dc7070d63ae36d95d
Summary:
The globbing algorithm is recursive and returns its own glob results merged
with its children's glob results. The merging is done by simply copying the
children's glob result and returning it. What this means is that a single
GlobResult will be copied K times, with K being the recursion depth at which it
was created. This makes the total number of copies be O(K*N) with N the result
length.
Since we can simply avoid these copies by simply creating the GlobResult in a
shared vector, we can avoid the copies entirely at the expense of taking a
lock.
Reviewed By: chadaustin
Differential Revision: D31288036
fbshipit-source-id: ae8a98a01eab2ba7f23908d347d7a4ec199cdfab
Summary:
In the case where a child is already loaded, this allows the code to not
allocate a SemiFuture, which should benefit code that repeatly calls it on
already loaded inodes. One typical example is the globbing entry point that
needs to find the TreeInode where the glob needs to be applied on.
Reviewed By: chadaustin
Differential Revision: D31283398
fbshipit-source-id: 76f82d74f2a45ee2b3b9bf442d47c0a2262bced9
Summary:
One layer about getChildRecursive is getInode, let's make this one use an
ImmediateFuture too.
Reviewed By: chadaustin
Differential Revision: D31283397
fbshipit-source-id: 8bc524bea857d6ec5bc045d6e3383d38133c3b38
Summary:
Vendoring this patch: https://github.com/curl/curl/pull/7737 to curl-sys rust crate. On windows the hg client is using curl that's bundled with sys-curl. I need this patch to have unix domain sockets working in hg client on windows.
I had to manually vendor https://raw.githubusercontent.com/mzr/curl/57e7ec4dbe4dd2831de51f2644879387d2ea7b44/docs/INSTALL because reindeer didn't do it. IDK why.
oss-eden-{darwin,linux,windows}-getdeps fail with:
```
FAILED: eden/scm/lib/backingstore/CMakeFiles/rust_backingstore.cargo eden/scm/lib/backingstore/debug/libbackingstore.a eden/scm/lib/backingstore/release/libbackingstore.a
cd /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E remove -f /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore/Cargo.lock && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E env CARGO_TARGET_DIR=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/eden/scm/lib/backingstore CARGO_HOME=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/_cargo_home cargo build --release -p backingstore --features fb
Blocking waiting for file lock on package cache
Blocking waiting for file lock on package cache
error: failed to calculate checksum of: /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL
Caused by:
failed to open file `/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL`
Caused by:
No such file or directory (os error 2)
```
Not idea how to fix this. Seems related to the fact that reindeer didn't vendor docs/INSTALL.
# EDIT:
# It might been caused by some bug in hg. now it's fine
# The failure in oss-eden-linux-getdeps looks unrelated (something with rocksdb)
Reviewed By: krallin
Differential Revision: D31370778
fbshipit-source-id: a1245f8cb6b58f5765e34c95dfd78325a8e6e457
Summary:
This diff adds snapshot tests for most eden api wire types, while at the same time making the testing code much smaller, including tests for "wire" and "serialize" roundtrips.
## Context:
The previous diff had added an easy way to add snapshot tests. This stack aims to simplify the wire protocol code needed to create/modify an endpoint. A good thing to do before that is to add snapshot tests to all wire types, so that if we change them in a refactor, we're confident they still work exactly the same. This will also be useful when a type is changed in the future.
## How this makes tests easier
- In order to create snapshot tests, we need example objects to test with. Luckily we already use a framework for generating example objects (quickcheck::Arbitrary), so the idea here is to use that to make snapshot tests as automatic as possible.
- At the same time, the "wire" and "serialize" roundtrip tests (which also used Arbitrary), can also be made more automatic.
Now, using a simple helper, `auto_wire_tests!(WireObjectName)`, it is possible to derive all three types of tests automatically. This makes the current code smaller, and safer as we now have the additional safety provided by snapshot tests.
## Observations
- Not all wire types had tests implemented for them (I assume because it was too much work doing so, and might have done that myself in the past), I only moved the ones that already had. I'll do another pass and add remaining objects on a following diff.
- There are a couple actual non-refactor changes. I'll add comments explaining those.
- quickcheck crate is using quite an old version. I tried updating but it snowballed into something much more complicated, so I kept using the old version. We'll need to get to it at some point, though.
Reviewed By: markbt
Differential Revision: D31019233
fbshipit-source-id: 30c4a90848d0a5dcaffb89b9a0cd1cebfe4ace55
Summary:
This is fairly mechanical diff that finalizes split of Hash into ObjectId and Hash20.
More specifically this diff does two things:
* Replaces `Hash` with `Hash20`
* Removes alias `using Hash = Hash20`
Reviewed By: chadaustin
Differential Revision: D31324202
fbshipit-source-id: 780b6d2a422ddf6d0f3cfc91e3e70ad10ebaa8b4
Summary:
A spawner type is required for new thrift clients, specify the noop one for now.
This also requires regenerating the generated thrift libraries.
Reviewed By: yancouto
Differential Revision: D31338518
fbshipit-source-id: cbecf3ec6f9678918ca459c19f1cc160214fadfd
Summary:
With the lookup processor now returning an ImmediateFuture, we can focus on its
caller, starting with getChildRecursive.
Reviewed By: chadaustin
Differential Revision: D31283396
fbshipit-source-id: 97abc57b9efe3540c5770aa952995c257e6eda4b
Summary:
While profiling glob queries sent by Buck, I noticed that EdenFS spends almost
as much time resolving the inode to perform the glob on as EdenFS takes to
actually perform the glob. Futures related overhead shows up as predominent,
thus let's convert these to ImmediateFuture to speed this up.
Reviewed By: chadaustin
Differential Revision: D31283395
fbshipit-source-id: 7355ddf7498f722ed8ec2989f010a28fb15c293f
Summary:
While std::variant is convenient, they are both slow to compile, and the
compiler cannot optimize it as well as a manually written tagged union. Since
ImmediateFuture is performance critical for EdenFS, let's use a tagged union
and speed them up by an additional 40%.
Reviewed By: chadaustin
Differential Revision: D31272296
fbshipit-source-id: e34be4489a596d3577b3bd900a1f20d6c7d8b693
Summary:
The max duration would cause UBSAN failures due to folly's SemiFuture code
multiplying the value which understandably cannot be represented. Splitting the
function is easy and avoids the problem entirely.
Reviewed By: genevievehelsel
Differential Revision: D31272297
fbshipit-source-id: c15ca70ad771c11b4f68bb9974422c0986d4928b
Summary:
When Buck is using the EdenFS globber, the searchRoot argument is set, thus
let's add a new argument to the benchmark to simulate a Buck workflow.
Reviewed By: chadaustin, genevievehelsel
Differential Revision: D31283399
fbshipit-source-id: 5e32b2aceb6090e26e88cf7f0d163448d56107d4
Summary:
The goal of this stack is to remove Proxy Hash type, but to achieve that we need first to address some tech debt in Eden codebase.
For the long time EdenFs had single Hash type that was used for many different use cases.
One of major uses for Hash type is identifies internal EdenFs objects such as blobs, trees, and others.
We seem to reach agreement that we need a different type for those identifiers, so we introduce separate ObjectId type in this diff to denote new identifier type and replace _some_ usage of Hash with ObjectId.
We still retain original Hash type for other use cases.
Roughly speaking, this is how this diff separates between Hash and ObjectId:
**ObjectId**:
* Everything that is stored in local store(blobs, trees, commits)
**Hash20**:
* Explicit hashes(Sha1 of the blob)
* Hg identifiers: manifest id and blob hg ig
For now, in this diff ObjectId has exactly same content as Hash, but this will change in the future diffs. Doing this way allows to keep diff size manageable, while migrating to new ObjectId right away would produce insanely large diff that would be both hard to make and review.
There are few more things that needs to be done before we can get to the meat of removing proxy hashes:
1) Replace include Hash.h with ObjectId.h where needed
2) Remove Hash type, explicitly rename rest of Hash usages to Hash20
3) Modify content of ObjectId to support new use cases
4) Modify serialized metadata and possibly other places that assume ObjectId size is fixed and equal to Hash20 size
Reviewed By: chadaustin
Differential Revision: D31316477
fbshipit-source-id: 0d5e4460a461bcaac6b9fd884517e129aeaf4baf
Summary: I'm parsing some deeply-nested JSON, and it's running into the limit. This feature enables a potential footgun, but even with the feature enabled you have to add code to reach for said footgun.
Reviewed By: jsgf
Differential Revision: D31284743
fbshipit-source-id: 00ea5d7d7db8bdeb878d48fe390831f39e007409
Summary: This diff fixes some integration test errors after enabling the new edenfsctl.
Reviewed By: xavierd
Differential Revision: D30789741
fbshipit-source-id: 02d74defc41def4fb6ea0cc4694f944b4c0044e2
Summary:
Some detail polishing.
Incomplete commands are commented out. Help messages are now printed correctly. Fixed a small behavior divergence in `eden config` (`to_string_pretty` uses multi-line string instead of escaping characters).
Reviewed By: xavierd
Differential Revision: D30547011
fbshipit-source-id: 98d323744ce7a7fc989cbf79dd07ed8af3cee09d
Summary: This diff adds the Rust edenfsctl to our open source build.
Reviewed By: xavierd
Differential Revision: D30788685
fbshipit-source-id: 603caa933ecfc5af0ede7e22f6c7911462da3a65
Summary:
Functions that only takes boolean arguments are fairly confusing and error
prone. Here, since we only ever pass a single true value to it, we could simply
inline setting the right counter in the caller. This makes the code easier to
read, and less error prone.
Reviewed By: genevievehelsel
Differential Revision: D31188413
fbshipit-source-id: 64c019ff52b1ff5644e5bea11a361e586044403f
Summary:
In advance of Thrift servers defaulting the queue timeout to 100 ms,
which is quite low for EdenFS's needs, explicitly set our queue
timeout to 5 seconds.
Reviewed By: zhengchaol
Differential Revision: D31218348
fbshipit-source-id: 35a109fb6848f7c81c4b58d70e2beae90557e1c8
Summary: we can just use getBackingStores like how is done for `startRecordingBackingStoreFetch` and only record non-empty fileAccesses. This will enable fetch logging for LocalCacheBackingStores which use an HgQueuedBackingStore under the hood
Reviewed By: zhengchaol
Differential Revision: D31215109
fbshipit-source-id: 443d28a57144fdcf078bd653ecf5726825f55740
Summary: fix the dynamic casting for getting a tracebus for the trace hg entrypoint. dynamic cast still makes sense at this point since `trace hg` should only be called on hg backed mounts
Reviewed By: chadaustin
Differential Revision: D31214737
fbshipit-source-id: 65e018e6658d934d8ecd3434bdfc3d72f6873d2b
Summary: instead of dynamic casting to find the repo name, all backing stores can return an optional reponame, and can check if the optional is set.
Reviewed By: zhengchaol
Differential Revision: D31214723
fbshipit-source-id: 9d10114ff6bde13254d3a3caaf2401f87d07ffd7
Summary: add more information to the runtime error thrown by the dynamic cast failure in `eden trace hg` and predictive fetch
Reviewed By: zhengchaol
Differential Revision: D31212247
fbshipit-source-id: 982901dfd2eb05db9ca6e7366277a07b6b29872f
Summary:
VC++ 2019 is pickier about which standard library includes include
each other. Be explicit.
Reviewed By: zhengchaol
Differential Revision: D31186916
fbshipit-source-id: 95cfa8848d0e2e312e2024923fa166db5f68dde0
Summary:
While debugging the unlinked inode unloading for NFS I have re-added these
logs a couple times. These seem valuable to have in eden so that we don't have
to add them any time we are debugging eden and we can debug a bit in a
production eden rather than dev built eden.
Reviewed By: xavierd
Differential Revision: D30971151
fbshipit-source-id: 58172079dfe4f4e4ba31bae30bf982e2cbe0fd29
Summary:
We run periodic inode unloading for unlinked inodes on NFS because we get no
information from the client on when inodes are no longer needed, and we have to
clean them up at some point for memory and disk reasons. See previous commit
summaries for more details on this (D30144901 (ffa558bf84)).
Let's add some counters on this so we have a bit more visibility into the
process. This counter is meant to mimic the PeriodicUnloadCounter counter.
Reviewed By: chadaustin
Differential Revision: D30966688
fbshipit-source-id: cfc8d769b53073d9f4c0c27b6bee20e222c6c8d2
Summary:
There is no need to read from the LocalStore twice, the tree is either present
in it, or not.
Reviewed By: chadaustin
Differential Revision: D31187972
fbshipit-source-id: 15bdeef9176b51e6ba3f62ed16550032b0024b94
Summary:
Some of EdenFS backing store requires EdenFS to cache objects locally to avoid
potentially expensive network fetches, while others already have some form of
local caching. In the past, all backing store fell in the first category, but
thanks to Mercurial's native backing store implementation the LocalStore
caching has become pure overhead for it. Previously, this was worked around by
configuring the LocalStore to not cache blobs locally, but this wasn't done for
trees. This config also conflicts with the need to cache blobs and trees
locally for backing stores in the first category (such as ReCas).
Since we know at construction time what backing store needs local caching, we
can simply wrap these in the newly introduced LocalStoreCachedBackingStore
store.
For now, since the Mercurial backing store always writes a proxy hash to the
LocalStore, bypassing the LocalStore for trees would be a regression due to the
added disk IO. Once proxy hashes are gone for Mercurial, we can remove the
LocalStoreCachedBackingStore wrapper.
Reviewed By: chadaustin
Differential Revision: D31118905
fbshipit-source-id: 4a2958eafeeb8144ee4421ec44dbd30cedceee29
Summary:
Now that we might have multiple kernel protocols per mount (i.e. both fuse and
nfs on macOS) let's include them in eden rage.
Reviewed By: xavierd
Differential Revision: D31154042
fbshipit-source-id: 38e7630829d70fe9dd6dbeabacc3b538ee798e0d
Summary:
The `ObjectFetchContext::Origin::FromBackingStore` is widely interpreted as
meaning that a network fetch was performed, but for some backing stores, this
isn't true. The Mercurial backing store for instance can either read data from
its on-disk cache, or from the network. Since both have very different
characteristics we shouldn't bundle them in the same enum value.
Since the backing store knows how data was obtained, let's have the backing
store return how it was obtained to enable the ObjectStore to properly record
this information. The `FromBackingStore` is also renamed to make it clearer
what its purpose is.
Reviewed By: zhengchaol
Differential Revision: D31118906
fbshipit-source-id: ee42a0c9d221f870742de07c0df7c732bc79d880
Summary:
we are passing some bytes into Popen and shlex.quote. shlex.quote expects a
string not bytes. fsencode gives us bytes fsdecode gives us string. Let's used
fsdecode instead.
Reviewed By: zhengchaol
Differential Revision: D31129335
fbshipit-source-id: 7792bdcd4dd833a4946daf8ec75576cfe4fc24af
Summary:
The duality of this function is a bit awkward, especially for backing stores
that want metadata caching, but not blob caching. This makes the code in
ObjectStore more complicated that it needs to be.
This will also be used in a future diff.
Reviewed By: chadaustin
Differential Revision: D31090782
fbshipit-source-id: cb7d7fc44d8780f86abad166d0f099675d29e5e7
Summary:
Similarly to blobs, fetching trees is converting data from HgImportRequest to
individual hashes and proxy hashes vector, making copies of these. This is
inefficient and it makes the code harder to read and understand. Passing the
batch of HgImportRequest directly avoid these copies and makes the code easier
to read.
Reviewed By: fanzeyi
Differential Revision: D30583567
fbshipit-source-id: e85952975141c92f9524095c62418baabf8fefcd
Summary:
Both the caller and the function itself are copying data to make sure that the
next function gets the data in the format it desires. This makes the code
complicated to read as well as inefficient as each conversion ends up being a
copy.
We can simply pass the HgImportRequest directly to avoid both of these issues
which removes all the copies.
A similar change will be done for the getTreeBatch function, after which the
HgDatapackStore will be folded onto the HgBackingStore code.
Reviewed By: fanzeyi
Differential Revision: D30563706
fbshipit-source-id: bb392f89e691c22ff9ad4df0d365ddb62077e657
Summary:
The intent behind the refresh method is to both read new data from the disk,
but also to flush the in-memory write buffer to disk. The name "flush" is used
in the revisionstore code to mean the latter, thus let's use "flush" in the
rest of the codebase.
Reviewed By: kmancini
Differential Revision: D30947873
fbshipit-source-id: c85a6abe770a47d6ce454d6af1fa73e505194a22
Summary: ACE will have to run in situations where Chef has not run, but we'll need to be able to reliably write to the auth logs so Blackbird can properly build detections. So we need these crates so we can build the somewhat foolproof solution to ensure ACE logs all executions.
Reviewed By: farnz
Differential Revision: D31066559
fbshipit-source-id: 9fa3b5778cd2602bdeaac90a9daa758b117babfe
Summary:
folly:format is deprecated in lieu of fmt and std::format. Migrate
most of EdenFS to fmt instead.
Differential Revision: D31025948
fbshipit-source-id: 82ed674d5e255ac129995b56bc8b9731a5fbf82e
Summary:
Backport: https://github.com/briansmith/ring/pull/1334
This will allow us to unpin Rust compiler to 1.53.0 and update to 1.55.0.
Reviewed By: xavierd
Differential Revision: D31039024
fbshipit-source-id: f6a9c918e836d93d03c34c77c12bbe63cf7cbe09
Summary:
EdenFS would never log anything when mounting via NFS, let's make it more
visible and easier to grep.
Reviewed By: chadaustin
Differential Revision: D31022158
fbshipit-source-id: 99fd3a04c90526eedf9951ac7c2bcd9e18ef8953
Summary:
this relies on local changes to make it so cargo metadata ACTUALLY finds this
binary: https://github.com/tokio-rs/console/pull/146 is where I try to upstream
it
Reviewed By: jsgf
Differential Revision: D30944630
fbshipit-source-id: 5d34a32a042f83eff7e7ae7445e23badf10fffe3
Summary: Watchman is a C++17 project now, so we can use std::optional.
Reviewed By: xavierd
Differential Revision: D30917549
fbshipit-source-id: 95d8ac15d4939a70347336ddfb120ab5025db993
Summary:
Having tons of booleans in a function can be very error prone from a caller
perspective, using a structure to pass in the same information can mitigate
some of this issue.
Reviewed By: kmancini
Differential Revision: D30883743
fbshipit-source-id: dcf38d29bfe2cb5155879f7ae4eab5cea31f798a
Summary:
During an `eden chown`, EdenFS will try to chown both the repository, and the
redirections. In some cases, chowning the redirection can both take a long time
and be unecessary. Consider the case where some automation temporarily chown a
repository to a service user that needs to access the repository, and then
chown it back to the owner of the repository. In that case, changing the
ownership of the redirection is superfluous and unecessary.
Reviewed By: mrkmndz
Differential Revision: D31010912
fbshipit-source-id: a882948005ac4fe29ff465088f196e0fc2bc10be
Summary:
In D29940980 (2e2b9755cf) we used shlex for a redirect subprocess command line.
The list does not always contain strings tho, which is a requirement to use
shlex.quote my guess is that they are paths. We should still str things
before we shlex.quote them.
Differential Revision: D31001622
fbshipit-source-id: 2a270781d7f2d84ad7a9a2f9975500b29306cfa8
Summary:
We periodically need to dereference inodes on NFS because we get no other info
from the kernel on when should dereference them.
This means the NFS kernel might have references to inodes after we delete them.
An unknown inode number is not a bug on NFS. It's just stale, so the error should
reflect that.
Reviewed By: xavierd
Differential Revision: D30144898
fbshipit-source-id: 3d448e94aea5acb02908ea443bcf3adae80eb975
Summary:
We periodically need to dereference inodes on NFS because we get no other info
from the kernel on when should dereference them.
It can be disruptive to a users workflow because an open files that were rm'ed
or removed on checkout will no longer have their old content. (on a native
filesystem or fuse applications that had the file open propr to the removal
would still be able to access files.) For most editors this is not a problem
because they read the file on open (seems fine for vim and vscode from testing).
However folks could theoretically have a workflow this does not jive with.
Let's make it configurable how often this runs, so users can control how
much we distupt their workflow.
Reviewed By: xavierd
Differential Revision: D30144899
fbshipit-source-id: 59cf5faea70b3aea216ca2bcb45b96e34f5e72b5
