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

Allow using buck without requiring a prelude cell #16

Closed
wants to merge 189 commits into from
Closed

Conversation

arlyon
Copy link
Contributor

@arlyon arlyon commented Sep 30, 2022

If building without a stub prelude, buck2 will complain with an error:

unknown cell alias: `prelude`. known aliases are: ``

This PR changes the autodiscovery code such that the prelude is optional.

facebook-github-bot and others added 30 commits September 27, 2022 10:07
fbshipit-source-id: 75941ca78d763a3b2fa289984193c454ad5b8ee1
Summary:
gRPC spec for "internal" says "Internal errors. This means that some invariants expected by the underlying system have been broken. This error code is reserved for serious errors."

I think "internal error" is reserved to lower-level errors, but not for user errors. And generally status codes should not be used for user errors.

Reviewed By: krallin

Differential Revision: D39799689

fbshipit-source-id: 8333bd5d3d582ab81f1f060874234282473b153c
Summary: No need to convert error to `CommandResult` twice. See, `error_to_command_result` is called in two places.

Reviewed By: krallin

Differential Revision: D39799732

fbshipit-source-id: 3570948e6aee874356d0127d656d8a4cfbe39171
Reviewed By: krallin

Differential Revision: D39799753

fbshipit-source-id: 9b35ae1606e24f8a1d940c8a0879d28a9ee90b3a
Summary: Extract `pump_events` function.

Reviewed By: krallin

Differential Revision: D39800088

fbshipit-source-id: cb4d4315dae9f9c9c733645e066cc02d421b8095
Reviewed By: krallin

Differential Revision: D39800116

fbshipit-source-id: 9bb2b0bd0b0f783f9afb6af9759af532a4f56d28
Summary: Reuse `result_to_command_result` function.

Reviewed By: krallin

Differential Revision: D39800164

fbshipit-source-id: 2f509d36a0f45b1794454e0fd4cff468e8bc0cbf
Summary:
I thought I fixed this test with D39773750 (dc9207d), but it seems I only made it pass for local executions (https://www.internalfb.com/intern/testinfra/diagnostics/4222124778920762.562950026994937.1664208083/).

It still seems to fail in CI. I'll disable for now, enable CI (next diff D39659530), and then debug the issue.

Reviewed By: KapJI

Differential Revision: D39853773

fbshipit-source-id: 783c4833169ee0e6d1d1812580f7ded9fa0a071d
Summary: This will allow the initial set of dice Data to read current state. i.e buckconfigs

Reviewed By: ndmitchell

Differential Revision: D39491412

fbshipit-source-id: 8caaa9d761a193efbb79e501ed72e2fd144d3103
Summary: used in D39491411

Reviewed By: ndmitchell

Differential Revision: D39491413

fbshipit-source-id: 27dcf41375f9d6d2075e5fa797ea0a5daaad0d52
Summary: When evaluation fails, make it more clear what is the path.

Reviewed By: krallin

Differential Revision: D39838582

fbshipit-source-id: 9a49f4f77c65f07df83a3a04a83526d9ef4390e3
Summary:
Better explicitly timeout in buck than have sandcastle job killing buck2 by timeout.

Note that failure to upload does not make buck2 to fail build, same as if curl fails: [source](https://fburl.com/code/qktiy3bk).

Context: investigating why buck2 hangs. This is not likely the issue, but rule it out.

Reviewed By: krallin

Differential Revision: D39793398

fbshipit-source-id: 06f17dca08d36b882ac6a6f8874883b0a3316a38
Summary:
* `cargo check` now checks this function on windows, which might be convenient to people working on windows
* no issues like unused import, unused enum variants on windows now

Reviewed By: krallin

Differential Revision: D39793419

fbshipit-source-id: be309ce40e96058c0347601d5a3885edf807e39f
Summary: A new test was added that does not pass for windows. (want to land this before landing D39659530)

Reviewed By: stepancheg

Differential Revision: D39860367

fbshipit-source-id: 8319d33526a23c725f45c27df25f459e04d04d38
Summary:
When working on fixing soft errors, make sure they don't disappear while iterating on fixes.

Reset in the beginning of command.

Which means, reset won't work correctly when commands are executed concurrently. It is fine.

Reviewed By: ndmitchell

Differential Revision: D39834245

fbshipit-source-id: 6aa3ee4a1aef80f0f46cf5ffc94308fccfe7da1a
Summary: `Path` can be relative, and it is not clear, if it is relative, it is relative to what.

Reviewed By: krallin

Differential Revision: D39799145

fbshipit-source-id: 57aea804af52a3246b79783b0c4ef204e1c8c504
Summary: There's [another copy of NullEventSink](https://fburl.com/code/fg6evw2z).

Reviewed By: krallin

Differential Revision: D39799381

fbshipit-source-id: 95a7e5de8ce8dc39a96d95f68d656cda1451e6dc
Reviewed By: krallin

Differential Revision: D39799465

fbshipit-source-id: 3596237619a8e299ba0281e4f2cbc8366311266f
Reviewed By: krallin

Differential Revision: D39799504

fbshipit-source-id: d738768eae1a28057bff1d862341e2643d21142f
Summary:
Delegate serialization to `Serialize` for `PathBuf`.

Practically should have no effect, because `derive(Serde)` for `AbsPathBuf` called `serialize_newtype_struct` which simply [serializes content in serde-json](https://github.com/serde-rs/json/blob/94019a31c6036dc4ebb9afc44a214f950caf0d1f/src/ser.rs#L260).

Done for the following diff D39841224 which implements `Deserialize`.

Note `Serialize` for `PathBuf` [serializes to string](https://github.com/serde-rs/serde/blob/649a72a58714b1a8223b3358b37f2f35b573f89e/serde/src/ser/impls.rs#L830), so this diff makes serialization of `AbsPathBuf` consistent with `PathBuf`.

Reviewed By: krallin

Differential Revision: D39841223

fbshipit-source-id: 54f633f9874a3480f1d6f395ba99d6c644798a0c
Summary: For the following diff D39841222.

Reviewed By: krallin

Differential Revision: D39841224

fbshipit-source-id: 54f27684d15d1753b7feaabaaf255dcab095d7aa
Summary: Use types to assert path is absolute.

Reviewed By: krallin

Differential Revision: D39841222

fbshipit-source-id: 90739eda5e231c389b1975ffc3e783c1c8cb8891
Summary:
`buck2 --no-buckd` changes directory to project root.

So we should stop calling `current_dir()` in random places. We should grab it once, store in some context, and fetch it from there where needed.

Reviewed By: krallin

Differential Revision: D39841368

fbshipit-source-id: 5f2693279be36a4681bdd88c404240d31c4c1c60
Summary: `buck2 --no-buckd` changes current directory. So avoid calling `current_dir`.

Reviewed By: krallin

Differential Revision: D39841414

fbshipit-source-id: c83e9a8e350fdb80651ba8fcd09f78b3f4609201
Summary: Cleanup.

Reviewed By: krallin

Differential Revision: D39841465

fbshipit-source-id: 8be22da7e3abd2b3417817d7fa3b73aa6a805147
Summary: More precise type.

Reviewed By: krallin

Differential Revision: D39841570

fbshipit-source-id: 74451363477c2dab9f82a3503b0bc9de5f02f05b
Summary: Types as documentation.

Reviewed By: krallin

Differential Revision: D39841822

fbshipit-source-id: 3a7d751253e87be96c363a8d876d492c2e963429
Summary: Avoid hard to debug issues of multithreaded programs changing directory.

Reviewed By: krallin

Differential Revision: D39841917

fbshipit-source-id: 9ecec66778347f65bb8d7f48d2cd414bb879c0e3
Summary: Avoid troubles with other threads may assume directory does not change.

Reviewed By: krallin

Differential Revision: D39842423

fbshipit-source-id: b22f1711859c69f1e6595461988209d215b7a8c3
Summary: No longer needed.

Reviewed By: krallin

Differential Revision: D39842473

fbshipit-source-id: 0b26f4e6dfe9935e0101c94687f036dd11dc55a9
stepancheg and others added 23 commits September 29, 2022 14:12
Summary: Dead code implemented incorrectly (see the comment in deleted code).

Reviewed By: lmvasquezg

Differential Revision: D39909294

fbshipit-source-id: e73c083161d0f5ddae58434a933b463a2507807d
Summary:
`TempPath::new`
* generates a path in temp directory
* does not create anything
* removes the path on drop

`tempfile` crate does not provide such utility.

Differential Revision: D39931909

fbshipit-source-id: e9b2d0fca371fd5b334bd5bb85a7c68b2a44a470
Summary:
* rename tests so they start with tested function names (except tests which test more than one function, and we should not write such tests)
* make assertions more explicit

Reviewed By: shonaganuma

Differential Revision: D39940675

fbshipit-source-id: 0eef1ede28d735e42a5e589bfc6dad28ef025387
Summary:
* D39931910 (67bdedb) adds `remove_all` function, and this test makes it clear they are not equivalent
* tests are useful, even if function is trivial and just delegates to `fs::remove_dir_all`, that function documentation [does not explain behavior precisely enough](https://doc.rust-lang.org/std/fs/fn.remove_dir_all.html)

Reviewed By: shonaganuma

Differential Revision: D39940771

fbshipit-source-id: 60b775f7faca64a92ce124be6689b4505dbf9631
Summary:
No need to create temporary directory to create a symlink.

(This does not fix the issue with long path, but this is intermediate step.)

Differential Revision: D39932720

fbshipit-source-id: b3ddbed6f44df1899a84be8da83787010699f471
Summary:
No need to create temp directory to create temp symlink.

(This does not fix the issue with long path, but this is intermediate step.)

Differential Revision: D39932719

fbshipit-source-id: f91e99e777260b998ff5157f49c676970919b467
Differential Revision: D39932946

fbshipit-source-id: f29c8989d2c1e5d77200e7ec18196d99e1db71f0
Summary: Used in the following diff D39937484.

Differential Revision: D39935238

fbshipit-source-id: 757c9394079a0a181d14b5ada63545b1757b71dc
Summary:
The function returns `~/.buck/tmp` created with old files removed.

This is used in the following diffs D39937646, D39937703 to create short symlinks to work with unix sockets.

Differential Revision: D39937484

fbshipit-source-id: 63af77c48e2b1b894c2c8cfb7b8f55d9e37e7029
Summary: Used in the following diffs D39937646, D39937703 to create short symlinks.

Differential Revision: D39937578

fbshipit-source-id: 1449d12241f1459b387bc75fa04a8d9e1d72e45b
Summary: Make sure unix socket path we connect to is shorter than 108 regardless of the length of `$TMPDIR` or real path of unix socket.

Differential Revision: D39937646

fbshipit-source-id: 3c79836e206a57ce51237d3f10e1c64ece3e6654
Summary: Make sure unix socket path we connect to is shorter than 108 regardless of the length of `$TMPDIR` or real path of unix socket.

Differential Revision: D39937703

fbshipit-source-id: e8209f40e6f4004d48c73d9ee0160a71e422290f
Summary: They are in Python 3, so should add them in Starlark too. Just the AST changes first, planning to add the parser/checking later.

Reviewed By: stepancheg

Differential Revision: D39945469

fbshipit-source-id: 4c1061f3bcaa355c4dfb423f589aa92fdafc92f1
Summary: Accidentally included this line in https://www.internalfb.com/diff/D39781567 (47ae62b), removing it for real since it is actually dead.

Reviewed By: stepancheg

Differential Revision: D39936487

fbshipit-source-id: 0adbedcc88b5e4aa6cdd4296c71b913a0433be36
Summary: Talks a lot about how we don't return an Err, when we do.

Reviewed By: IanChilds

Differential Revision: D39962875

fbshipit-source-id: 069dc7b7d6b867308dbdd475d81742c77bd803c5
Summary: Like the title says, adapting this to work on windows aswell. Also removing noop test since now we have one workign test on the file

Reviewed By: krallin

Differential Revision: D39928991

fbshipit-source-id: 0daa7e48ac0447d72da5741ee54e3ecb1332eef0
Summary: Like the title says, adapting this to work on windows aswell.

Differential Revision: D39932979

fbshipit-source-id: c1544b16e52a8a0144f862937a6f6709bc069327
Summary:
In `buck1`, `manifest_entries` is a grab-bag of things that were added to the manifest when running `aapt`. However, ever since we migrated to `aapt2`, they are no longer added to the manifest, so their name is quite misleading. Some aspects of the `manifest_entries` are being used elsewhere (e.g. the min SDK version) but it would be better to have an explicit attribute to make it clearer what's going on.

Therefore, going to explicitly not support `manifest_entries`, and add a `min_sdk_version` attribute in another diff.

Reviewed By: ianlevesque

Differential Revision: D39885821

fbshipit-source-id: e7ae4b649524f111552989e98901ecd841797f31
Summary:
* deduplicate code
* update global IO counter

Reviewed By: lmvasquezg

Differential Revision: D39948894

fbshipit-source-id: 282ec2d5b60318aad28abba77c4c581b568f7a38
Summary: Avoid creating an anyhow we don't use.

Reviewed By: IanChilds

Differential Revision: D39962876

fbshipit-source-id: 7b1101aa0e3e9cc73ef97eb52066dd935fbff15f
Summary: Previously we threw away the error message, and made up our own, in two steps. That can be a little dangerous. Use map_err which is more clear we are substituting the error, which is more obviously a sensible thing to do.

Reviewed By: IanChilds

Differential Revision: D39962878

fbshipit-source-id: 5029433ec1814f99a11b821d0716f5acc12f0d21
Summary: Before we ignored them by taking the Result (which is must_use) and turning it to an option which we silently ignored. Now make it clear we are entirely ignoring the error. Would be ideal if we could explain why we were doing so, but at least it's now clear that we are doing so.

Reviewed By: IanChilds

Differential Revision: D39962874

fbshipit-source-id: 70149528e726177e2b18744149345a95fe1359fb
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 30, 2022
@arlyon arlyon changed the title allow using buck without requiring a prelude cell Allow using buck without requiring a prelude cell Sep 30, 2022
@facebook-github-bot
Copy link
Contributor

@arlyon has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@themarwhal themarwhal deleted the no-prelude branch August 24, 2023 18:09
facebook-github-bot pushed a commit that referenced this pull request Feb 15, 2024
Summary:
The stack around D42099161 introduced the ability to dynamically set the log level via a debug-command. This requires using `tracing_subscriber::reload::Layer`. The implementation of that machinery is backed by a `RwLock` [here](https://fburl.com/code/4xv0ihpn). This `RwLock` is read locked once on every single `#[instrumentation]` call to check whether the instrumentation is active or not.

On top of that, something in our fbsource third-party config enables the `parking_lot` feature of `tracing_subscriber`. This means that this is a `parking_lot::RwLock`, not a `std::sync::RwLock`, which is bad because it's well known that parking lot has problems with excessive spinning.

What all that adds up to is that when you put `#[instrument]` on a super hot function in dice, you get dozens of threads that are all simultaneously doing this:

```
  thread #23, name = 'buck2-rt', stop reason = signal SIGSTOP
    frame #0: 0x000000000c94a1fa buck2`<parking_lot::raw_rwlock::RawRwLock>::lock_shared_slow + 122
    frame #1: 0x0000000007a80b54 buck2`<tracing_subscriber::layer::layered::Layered<tracing_subscriber::reload::Layer<tracing_subscriber::filter::layer_filters::Filtered<tracing_subscriber::fmt::fmt_layer::Layer<tracing_subscriber::registry::sharded::Registry, tracing_subscriber::fmt::format::DefaultFields, tracing_subscriber::fmt::format::Format, std::io::stdio::stderr>, tracing_subscriber::filter::env::EnvFilter, tracing_subscriber::registry::sharded::Registry>, tracing_subscriber::registry::sharded::Registry>, tracing_subscriber::registry::sharded::Registry> as tracing_core::subscriber::Subscriber>::enabled + 292
    frame #2: 0x000000000a172d77 buck2`<dice::legacy::incremental::dep_trackers::internals::Dep<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>> as dice::legacy::incremental::graph::dependencies::Dependency>::recompute::{closure#0} + 183
    frame #3: 0x000000000a03606c buck2`<futures_util::stream::futures_unordered::FuturesUnordered<core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = core::result::Result<(alloc::boxed::Box<dyn dice::legacy::incremental::graph::dependencies::ComputedDependency>, alloc::sync::Arc<dyn dice::legacy::incremental::graph::GraphNodeDyn>), dice::api::error::DiceError>> + core::marker::Send>>> as futures_util::stream::stream::StreamExt>::poll_next_unpin + 444
    frame #4: 0x0000000009fc4755 buck2`<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::compute_whether_dependencies_changed::{closure#0} (.llvm.4229350879637282184) + 1733
    frame #5: 0x0000000009ef30d4 buck2`<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::compute_whether_versioned_dependencies_changed::{closure#0}::{closure#0} + 228
    frame #6: 0x0000000009f194ec buck2`<buck2_futures::cancellable_future::CancellableFuture<<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::new_dice_task::{closure#0}> as core::future::future::Future>::poll (.llvm.11181184606289051452) + 3500
    frame #7: 0x0000000009f04bbf buck2`<futures_util::future::future::map::Map<buck2_futures::cancellable_future::CancellableFuture<<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::new_dice_task::{closure#0}>, buck2_futures::spawn::spawn_inner<<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::new_dice_task::{closure#0}, dice::api::user_data::UserComputationData, futures_util::future::ready::Ready<()>>::{closure#0}> as core::future::future::Future>::poll + 31
    frame #8: 0x0000000009ff0339 buck2`<futures_util::future::future::flatten::Flatten<futures_util::future::future::Map<futures_util::future::ready::Ready<()>, buck2_futures::spawn::spawn_inner<<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::new_dice_task::{closure#0}, dice::api::user_data::UserComputationData, futures_util::future::ready::Ready<()>>::{closure#1}>, <futures_util::future::future::Map<futures_util::future::ready::Ready<()>, buck2_futures::spawn::spawn_inner<<dice::legacy::incremental::IncrementalEngine<dice::legacy::key::StoragePropertiesForKey<buck2_build_api::actions::calculation::BuildKey>>>::new_dice_task::{closure#0}, dice::api::user_data::UserComputationData, futures_util::future::ready::Ready<()>>::{closure#1}> as core::future::future::Future>::Output> as core::future::future::Future>::poll + 201
    frame #9: 0x00000000093f9f22 buck2`<tokio::task::task_local::TaskLocalFuture<buck2_events::dispatch::EventDispatcher, core::pin::Pin<alloc::boxed::Box<dyn core::future::future::Future<Output = alloc::boxed::Box<dyn core::any::Any + core::marker::Send>> + core::marker::Send>>> as core::future::future::Future>::poll + 130
    frame #10: 0x000000000939fdcb buck2`<tracing::instrument::Instrumented<<buck2_build_api::spawner::BuckSpawner as buck2_futures::spawner::Spawner<dice::api::user_data::UserComputationData>>::spawn::{closure#0}> as core::future::future::Future>::poll + 139
    frame #11: 0x00000000091ca5a9 buck2`<tokio::runtime::task::core::Core<tracing::instrument::Instrumented<<buck2_build_api::spawner::BuckSpawner as buck2_futures::spawner::Spawner<dice::api::user_data::UserComputationData>>::spawn::{closure#0}>, alloc::sync::Arc<tokio::runtime::scheduler::multi_thread::handle::Handle>>>::poll + 169
    frame #12: 0x00000000091c1b22 buck2`<tokio::runtime::task::harness::Harness<tracing::instrument::Instrumented<<buck2_build_api::spawner::BuckSpawner as buck2_futures::spawner::Spawner<dice::api::user_data::UserComputationData>>::spawn::{closure#0}>, alloc::sync::Arc<tokio::runtime::scheduler::multi_thread::handle::Handle>>>::poll + 146
    frame #13: 0x000000000c920f6f buck2`<tokio::runtime::scheduler::multi_thread::worker::Context>::run_task + 895
    frame #14: 0x000000000c91f847 buck2`<tokio::runtime::scheduler::multi_thread::worker::Context>::run + 2103
    frame #15: 0x000000000c932264 buck2`<tokio::runtime::context::scoped::Scoped<tokio::runtime::scheduler::Context>>::set::<tokio::runtime::scheduler::multi_thread::worker::run::{closure#0}::{closure#0}, ()> + 52
    frame #16: 0x000000000c932169 buck2`tokio::runtime::context::runtime::enter_runtime::<tokio::runtime::scheduler::multi_thread::worker::run::{closure#0}, ()> + 441
    frame #17: 0x000000000c91efa6 buck2`tokio::runtime::scheduler::multi_thread::worker::run + 70
    frame #18: 0x000000000c906a50 buck2`<tracing::instrument::Instrumented<tokio::runtime::blocking::task::BlockingTask<<tokio::runtime::scheduler::multi_thread::worker::Launch>::launch::{closure#0}>> as core::future::future::Future>::poll + 160
    frame #19: 0x000000000c8f8af9 buck2`<tokio::loom::std::unsafe_cell::UnsafeCell<tokio::runtime::task::core::Stage<tracing::instrument::Instrumented<tokio::runtime::blocking::task::BlockingTask<<tokio::runtime::scheduler::multi_thread::worker::Launch>::launch::{closure#0}>>>>>::with_mut::<core::task::poll::Poll<()>, <tokio::runtime::task::core::Core<tracing::instrument::Instrumented<tokio::runtime::blocking::task::BlockingTask<<tokio::runtime::scheduler::multi_thread::worker::Launch>::launch::{closure#0}>>, tokio::runtime::blocking::schedule::BlockingSchedule>>::poll::{closure#0}> + 153
    frame #20: 0x000000000c90166b buck2`<tokio::runtime::task::core::Core<tracing::instrument::Instrumented<tokio::runtime::blocking::task::BlockingTask<<tokio::runtime::scheduler::multi_thread::worker::Launch>::launch::{closure#0}>>, tokio::runtime::blocking::schedule::BlockingSchedule>>::poll + 43
    frame #21: 0x000000000c90d9b8 buck2`<tokio::runtime::task::harness::Harness<tracing::instrument::Instrumented<tokio::runtime::blocking::task::BlockingTask<<tokio::runtime::scheduler::multi_thread::worker::Launch>::launch::{closure#0}>>, tokio::runtime::blocking::schedule::BlockingSchedule>>::poll + 152
    frame #22: 0x000000000c90b848 buck2`<tokio::runtime::blocking::pool::Inner>::run + 216
    frame #23: 0x000000000c9002ab buck2`std::sys_common::backtrace::__rust_begin_short_backtrace::<<tokio::runtime::blocking::pool::Spawner>::spawn_thread::{closure#0}, ()> + 187
    frame #24: 0x000000000c90042e buck2`<<std::thread::Builder>::spawn_unchecked_<<tokio::runtime::blocking::pool::Spawner>::spawn_thread::{closure#0}, ()>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} + 158
    frame #25: 0x000000000757e4b5 buck2`std::sys::unix::thread::Thread::new::thread_start::h618b0b8e7bda0b2b + 37
    frame #26: 0x00007f2ba1e9abaf
```

This hit an open source user super hard over in #555 .

This diff approaches this issue by putting all the `#[instrument]`s in dice behind `#[cfg(debug_assertions)]`. This allows them to still be used in debug mode, but keeps them out of any hot paths. I do not have numbers to show that most of these matter. The only one I know matters for sure is `recompute`.

Alternatives are to either try and tailor this to the callsites we know are hot (that sounds error prone) or to revert the stack that inadvertently introduced the RwLock. Even if we did that though, it's probably still safer to keep `#[instrument]` out of super hot paths like this.

Reviewed By: stepancheg

Differential Revision: D53744041

fbshipit-source-id: 85bce9af2fec8ad1d50adc241d3b8e2bfc5cec87
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.