Improve error messages by adding additional context #5
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
Reviewed By: stepancheg Differential Revision: D38842384 fbshipit-source-id: 0981a1f7b09752c30fe99e0c134435b36a29d44d
Summary: Intercept and cleanup the flags before handing them over to fbcc. Clang driver can add certain flags that conflict with the existing bitcode attribute. For example, clang driver always tries to add `-triple`, `-target-cpu` and `-tune-cpu` even if they are not explicitly given. This could cause codegen discrepancy if the input bitcode was compiled with different values of these flags. Previously applied values are already encoded in the bitcode file as metadata, and we should just use them. Differential Revision: D38881137 fbshipit-source-id: 2e5be1e888372b0ebe7cae1829144c0f7d02ce03
Summary: When `-c fbcode.use_link_groups=True`, we link all static dependencies that go into shared libraries without archive semantics (i.e. use `--whole-archvie` to force load the dependency). This does not work when building a `python_binary` that links against `libomnibus.so`, we need to make sure that archive semantics is preserved for all dependencies that link into `libomnibus.so`. #forceTDHashing Reviewed By: luciang Differential Revision: D38915017 fbshipit-source-id: 3f0a33d26fe3ca99fc0c0a4b8db06eff2f5caa4c
Summary: Smaller `cli` crate, while `buck2_test` is compiled in parallel with `buck2_bxl` and `buck2_server`. Reviewed By: krallin Differential Revision: D38849510 fbshipit-source-id: 73d57ee2a34b3de77faab6bedebe4db833abd2ba
Summary: `bxl` command which is to be moved to `buck2_bxl` crate needs it. Reviewed By: krallin Differential Revision: D38850216 fbshipit-source-id: e0f6d003f9c7f172c2d8dc1524f5d7ddb0aafb70
Summary: Preparation to move `bxl` command to `buck2_bxl` crate (D38851147). Reviewed By: krallin Differential Revision: D38850215 fbshipit-source-id: fe3897e37de5fb8af037a93a47c1d651c9d1ce46
Summary: This utility is needed for both `build` and `bxl` commands. And it depends on `buck2_build_api`. `buck2_server_context` should not depend on `buck2_build_api`. So move this utility to `buck2_build_api`. Unfortunately, `buck2_build_api` now needs to depend on `cli_proto` just for single enum. Cannot find a better solution. (However, it does not affect compilation speed because `cli_proto -> buck2_events -> buck2_interpreter -> buck2_build_api`.) Reviewed By: krallin Differential Revision: D38850492 fbshipit-source-id: b9ecfc53644bf82bdcdcecc2a63d30dee3b55817
Summary: Smaller `cli` crate. Reviewed By: krallin Differential Revision: D38851147 fbshipit-source-id: 7ffa5f1bb9e01b61178cfcfae123d5702a2c899a
Summary: Make it easier to move files around. Reviewed By: krallin Differential Revision: D38852752 fbshipit-source-id: 19cecca3652e9af37414cb707f4ac65102c1ac01
Summary: Fewer cyclic dependencies between modules. Reviewed By: krallin Differential Revision: D38852924 fbshipit-source-id: c622a46316b69f66bb4ca77712b3be94d1c39504
Summary: Refactoring for future crate split. Reviewed By: krallin Differential Revision: D38853001 fbshipit-source-id: c2e839419c2a42ab48ea713b7c1c50cb5e64ed2c
Summary: Currently, the mergebase in WatchmanStats message is recorded only when it is on a fresh instance. It's useful it's recorded when non-fresh instance too. Reviewed By: bobyangyf Differential Revision: D38883055 fbshipit-source-id: e95babbe1288a972757d2dfcb70980d45d902bb3
Summary: The base revision on which the build happened is worth being recorded. Reviewed By: bobyangyf Differential Revision: D38884524 fbshipit-source-id: 5b81a0cc83aaac3b71a7a16af793057292d9ea51
Summary: Scribe capacity management. As far as I can tell, we send these events to Scribe but don't actually use them anywhere in Ingress. Since many of these start events end up including long uncompressed strings (configurations & targets), let's just not send them and see how much capacity we save. Differential Revision: D38923213 fbshipit-source-id: 9891cce930d99f6e80f147647263970e6c3b5fea
Summary: ^ Reviewed By: bobyangyf Differential Revision: D37666638 fbshipit-source-id: 26343e28532f963fff057d7047d01bf15ca8ac9c
Summary: We use an `Arc<ProjectRoot>` in some places and then clone `ProjectRoot` other places. Refactor `ProjectRoot` to make it `Dupe` by making it hold `Arc<AbsPathBuf>` instead of `AbsPathBuf`. Reviewed By: bobyangyf Differential Revision: D38925181 fbshipit-source-id: f035c3f857f038653210f56a99bdf3b0af06e878
Summary: `init_data` currently calls await on two async functions in sequence. I'm going to add more async work in a follow-up diff, so let's make invoking these async functions concurrent using a `try_join`. Reviewed By: bobyangyf Differential Revision: D38877430 fbshipit-source-id: 25eddd0cd38fe1b7029ac55edd8dae4a038c09f2
Summary: moveitmoveit Reviewed By: krallin Differential Revision: D38853127 fbshipit-source-id: 4cddcd86f0ca8ac4cc9240ea576b596dc2f4ec82
Reviewed By: krallin Differential Revision: D38853673 fbshipit-source-id: d229310c742157314e2832925fc5ca0244fa910c
Summary: Function is called once. Reviewed By: bobyangyf Differential Revision: D38896503 fbshipit-source-id: a2d2043543ce056f9370d070c95b21ce57e78c61
Summary: ^ Reviewed By: ndmitchell Differential Revision: D38936894 fbshipit-source-id: f95096050d7def8b1fbbfcb305fc31ea93e956fe
Summary:
`buck2_client` is a crate which does not depend on `buck2_build_api`.
`buck2_client` is compiled in parallel with `buck2_server`, `buck2_bxl`, `buck2_test`.
`buck2_client` should not depend on `buck2_server` and vice versa.
Large part of the client process of buck2 will go into this crate.
This diff moves `ExitResult` to `buck2_client`. Following diffs move more.
The goal of this project is faster incremental compilation (e.g. when single line is changed in `buck2_common` crate) which should increase developer productivity.
Dependency graph at the top of the stack:
{F762315131}
Reviewed By: krallin
Differential Revision: D38854060
fbshipit-source-id: 7acbd29d41363d18d7b0acc824ed7e03b57c839d
Reviewed By: krallin Differential Revision: D38854062 fbshipit-source-id: 4e3a1c828b09b18ddb2dcd39c1987724a0688e8b
Reviewed By: krallin Differential Revision: D38854067 fbshipit-source-id: 254c86ee9c331961322cedec5cfd71ae16f4eb23
Reviewed By: krallin Differential Revision: D38854059 fbshipit-source-id: fb8c42f62dc4c5adf83984da139a77a3874932a4
Summary: Sandcastle has a lot of messages like: ``` [2022-08-18T08:30:46.802-07:00] Waiting on daemon... [2022-08-18T08:30:53.802-07:00] Waiting on daemon... [2022-08-18T08:31:00.902-07:00] Waiting on daemon... [2022-08-18T08:31:07.902-07:00] Waiting on daemon... [2022-08-18T08:31:14.903-07:00] Waiting on daemon... [2022-08-18T08:31:22.002-07:00] Waiting on daemon... [2022-08-18T08:31:29.102-07:00] Waiting on daemon... [2022-08-18T08:31:36.202-07:00] Waiting on daemon... [2022-08-18T08:31:43.302-07:00] Waiting on daemon... [2022-08-18T08:31:50.402-07:00] Waiting on daemon... ``` Make it more clear we are waiting for buck2, and not any other process may be running on the same server: buck1, watchman, hg? Ideally together with this message we should print, what the daemon is doing. Differential Revision: D38932648 fbshipit-source-id: 63625044ebd98c3b9fec0a3e8142daac5feab1a5
Reviewed By: krallin Differential Revision: D38854063 fbshipit-source-id: b4ca2efac40a9d000a7f4b8141c249f8d7a6b310
Reviewed By: krallin Differential Revision: D38854061 fbshipit-source-id: fd65d730403af3e7a8c1335be7d427e4b11e1753
Reviewed By: krallin Differential Revision: D38854066 fbshipit-source-id: f788c84ee451a59cc4dcecc61c1d9685a72db96e
Reviewed By: krallin Differential Revision: D38854068 fbshipit-source-id: eaed04c1a189a8822660bc2b83b52cb0f128ec68
Summary: This is a useful bit of info and there's no reason not to log it. Reviewed By: ndmitchell Differential Revision: D39017540 fbshipit-source-id: 7572ca3f81678d908e4b9afe91cea33eba8c351a
Summary: Need this to implement corner case in `reuse-current-config` when there is no previous config to use. Previously, the behavior would be to [panic](https://www.internalfb.com/code/fbsource/[a171a3900c5ae7eff111480c5dc048f1372256e6]/fbcode/buck2/dice/dice/src/injected.rs?lines=45-51) if we tried to retrieve a nonexistent CellResolverKey from DICE (since CellResolverKey [implements InjectedKey](https://www.internalfb.com/code/fbsource/[a41862cda8ac6fb40f46bef6b973baf6114447ba]/fbcode/buck2/buck2_common/src/dice/cells.rs?lines=30)) Reviewed By: bobyangyf Differential Revision: D38974664 fbshipit-source-id: e57087bf61c43134ca94b6b44a3acb81ba3f91da
Summary: Extract part of `ProviderError` and move it next to where errors are used. Fewer dependencies between modules. Reviewed By: krallin Differential Revision: D38974834 fbshipit-source-id: a7dba0f953c6bb77d53f362b52aca70997358ee8
Summary: Move error next to where it is used. Fewer dependencies between modules. Reviewed By: krallin Differential Revision: D38974833 fbshipit-source-id: 863d6aefdc41512a56fa15ef4c31e2568b9514e6
Summary: Do not rely on ``` #[macro_use] extern crate starlark; ``` in `lib.rs` Make generated code self-contained. Reviewed By: krallin Differential Revision: D38974831 fbshipit-source-id: a5bce5468361cdcff1d21532d9e8ebb1767c154c
Summary: Fewer dependencies between modules. (Context: trying to split `buck2_build_api`). Reviewed By: krallin Differential Revision: D38974832 fbshipit-source-id: cde85eaa115b32594e3eaca95fffe5649b869f3c
Summary: Easier to figure out what is used where. For example, now it is clear that `directory_to_re_tree` is only called in tests. Also remove dead code. Reviewed By: krallin Differential Revision: D38979693 fbshipit-source-id: 13222ea69fda9a9542ad8037404eeb505b614e75
Summary: In current dist_lto implementation, we build a dependencies list ourselves and wrap them with "start-lib/end-lib" in final_link stage, Which we know may not have the correct order. with the newly added flag `"-Wl,--thinlto-full-index"` we can ask linker to produce a full list of dependencies in index stage, that can be our source of truth. In this diff we swapped the input for final_link to use the above full index file. to make it work to fbcode we also did 2 extra things: 1. dist_lto copying files around to work around a buck2 issue (BG: https://fb.workplace.com/groups/buck2users/permalink/3216781525244873/). so we need to modify the index file a little (only for those need opt stage) to reflect the correct location. 2. we use linker wrapper which will add extra dependencies in both index and final_link stage, to avoid double adding we removed these deps from the index files, they will be added back in final_link stage. it's something maybe we can improve in future: T130322878 Reviewed By: apolloww Differential Revision: D38959206 fbshipit-source-id: 46baa1428f1525cf480779e0d55625114116d79e
Summary: Make sure that documentation generation exposes the starlark-rust/docs dir. This is done as a copy into docs/generated because docusaurus really didn't like traversing upward. Reviewed By: ndmitchell Differential Revision: D39027676 fbshipit-source-id: 1d4f53a7a61d9a2b41055c414a712adfd32edd51
Summary: Original commit changeset: 46baa1428f15 Original Phabricator Diff: D38959206 Reviewed By: apolloww Differential Revision: D39037519 fbshipit-source-id: 240726bb4db3757e89b4e1d5b0b57d5b085331ce
Summary: Up the stack, we use tsets to replace both the resource graph and linkable graph. When using tsets, we'll need to use traverse to get all nodes (a reduction which generates a map uses more memory), which returns a list of nodes. Both link and resource groups need mappings of label -> nodes to process. Previously this was obtained through `graph.nodes`. To prepare for getting this data from tsets, update the APIs that took a graph object to instead taking a dict of label -> node. Reviewed By: d16r Differential Revision: D38954219 fbshipit-source-id: 3c7373a1a138bd0f6fab047b01f9d029dfba01b9
Summary: As the title says, use a tset for ResourceGraph. We use a traverse to get all nodes. Combined with the use of tsets for LinkableGraph, this reduces memory consumption for large apps. Numbers and perf evaluates at the end. Reviewed By: d16r Differential Revision: D38955685 fbshipit-source-id: 68c8c408209716bb06160d2af5722f079c7ae23c
Summary: Move `buck2_build_api::deferred` to submodule `types`. Modules without submodules are: * easier to move around * no hidden dependencies for submodules About the latter: ``` use foo::Bar; mod baz; ``` In `baz`, `Bar` is available. Making tracking dependencies harder. Reviewed By: ndmitchell Differential Revision: D38981280 fbshipit-source-id: ec2e8b14ff20205d946ceb4065cbbdb0124b7768
Summary: Not needed, keep impl for `IncrementalEngine`. Reviewed By: bobyangyf Differential Revision: D39031597 fbshipit-source-id: 26258e7bacdf8683c23b6cc41965cfce094f6014
Summary: to index, key type, key description. More convenient this way, because in the output key types are often repeated. Reviewed By: bobyangyf Differential Revision: D39039436 fbshipit-source-id: 5cfd9994cea0359bef51ac88ccfdf39c62fb4edb
Summary: Cleanup. Reviewed By: krallin Differential Revision: D39046575 fbshipit-source-id: 242fa0ba5181f9cfcfb701830c1816c3e1287f34
Summary: Make the prelude more reusable. #dontscheduledeferredjobs Reviewed By: milend Differential Revision: D38152463 fbshipit-source-id: eb55457d27445d7fd6efe7ba67693f71ccb78d9e
Summary: Whoops. Reviewed By: stepancheg Differential Revision: D39050770 fbshipit-source-id: 91b9ed2cb30c62c4beb472145ad93318676b67c7
Summary: Pass `ArticfactPath` instead of `Artifact` (and like) only to obtain `ArtifactPath` by calling `get_path`. `Artifact` depends on actions, and `ArtifactPath` does not. Trying to break dependency cycles in `buck2_build_api`. Also I'd argue, code is cleaner now: we know that resolved path only depends on `ArtifactPath` and on nothing else. Reviewed By: krallin Differential Revision: D39048849 fbshipit-source-id: 488619a4d4cc23623eeb1ab79cf5fc74a7330863
Summary: Accept `BuckOutPath` instead of `BuildArtifact` because `BuckOutPath` is all we need. More clear, and untangling dependencies: `BuildArtifact` depends on actions, but `BuckOutPath` does not. Reviewed By: krallin Differential Revision: D39048848 fbshipit-source-id: 2ad604df2346dab96a2222e3e65b383b6ea0a1e4
Summary: Following diff D39048846 makes `BuckOutPath` `Arc` to make it `Dupe`. Reviewed By: krallin Differential Revision: D39048847 fbshipit-source-id: 45c20f90412e41acf826dbe645d15c074b28b625
Summary: For the following diff D39048850 which replaces `BuildArtifact` with `BuckOutPath` in `CommandExecutor`. Reviewed By: krallin Differential Revision: D39048846 fbshipit-source-id: 8538bef59ab98176c8ab84a0c91e499d6c6adffb
Summary: Otherwise we fail to build, see https://fb.workplace.com/groups/buck2users/posts/3220993748156984/ Reviewed By: krallin Differential Revision: D39053334 fbshipit-source-id: 56fc015b812f949ca863ac4d8ff2e03a678569f3
facebook-github-bot
added
the
CLA Signed
facebook-github-bot
force-pushed
the
main
branch
from
c53c46c to
4f0e2ec
Compare
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
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.