Adds some basic quickstart tutorials following the examples set in the prelude and no-prelude folders #3
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
Summary: Certain cases - e.g. `third-party/pypi/numpy:_multiarray_umath` Have interdependencies between object files. Renaming defined symbols in one compilation unit without updating the references in another resulted in missing symbol errors. Differential Revision: D38626181 fbshipit-source-id: 9bf88fc427ffe80eabf05da74247fc263460a34d
Summary:
This makes it possible for a `rust_binary` or `cpp_binary` which depends transitively on a `rust_library` to access resources declared by the dependency.
Per {D38078866 (facebook/buck2@8433a0e)}, we are using the exact same buck2 provider type for resources of a `cpp_library` as for resources of a `rust_library`, so there is no change needed in this diff to anything under buck2/prelude/cxx. The existing code in there will automatically pick up the resources that this diff makes `rust_library` provide.
Reviewed By: zertosh
Differential Revision: D38117486
fbshipit-source-id: a521924c86cd82968a6d332b6307344d4858d246
Summary: Used later (D38681490). Also may be helpful when debugging. Reviewed By: milend Differential Revision: D38680885 fbshipit-source-id: 724f8dc4773808583c478414a4dcfd50e343a789
Summary: Release announcement: https://blog.rust-lang.org/2022/08/11/Rust-1.63.0.html Release notes: https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1630-2022-08-11 Reviewed By: zertosh Differential Revision: D38604755 fbshipit-source-id: c52bcbe85302cdd6d13d561f6620b7a1ac72920e
Summary: The type has been switched with D38631414 (facebook/buck2@43ffe8a) but then restored back with D38620520 (facebook/buck2@98e71ee). Switching it again to `label()` Reviewed By: chatura-atapattu Differential Revision: D38715560 fbshipit-source-id: e8282e2a3abb1b1e04a6cbc64d40a0627bcac682
…ping Summary: We currently don't inform users if there are no matches for a mapping or link group (e.g. due to a typo in the pattern string). This may lead to unexpected behavior for the user as the .so file will still be created but won't have the contents that they expect. Based on offline discussion with Christy, we will just warn the user if there is a partial match (as parts of our default configs may not be relevant to their binary) but error out if there is no match for a link group. Reviewed By: chatura-atapattu Differential Revision: D38692036 fbshipit-source-id: 1360c38eba5eaabebfcc25ff1a46e344d808a3e4
Summary: Switch `InstallInfo`'s `installer` type from `RunInfo` to `label`. This change will allow to compare installers between each other. This change unblocks grouping InstallerInfo over the same installer app. (Will add in the next diff) Reviewed By: lebentle Differential Revision: D38219113 fbshipit-source-id: 9763fb82f30eeb36fa93b0e102170e8e3304fcae
Summary: Add `list()` operation for BXL filesystem. This version of `list()` will ignore ignored files by default. Reviewed By: bobyangyf Differential Revision: D38703953 fbshipit-source-id: fcb31b61a8398466300a6cec1c3fb2702bc47769
Summary: Before this diff "Too many recursion levels" error was used for both: * call stack overflow * equality/comparison evaluation stack overflow Change call stack overflow error to "Starlark call stack overflow" to make error easier to investigate. Reviewed By: bobyangyf Differential Revision: D38705604 fbshipit-source-id: d9cde9799ba1f51eb9c41011d62f18e5a083acb3
Reviewed By: bobyangyf Differential Revision: D38715389 fbshipit-source-id: 500a28438ffe83f58b8e6cbe938347035a39eda0
… link group or mapping Differential Revision: D38692036 (facebook/buck2@5e4656b) Original commit changeset: 1360c38eba5e Original Phabricator Diff: D38692036 (facebook/buck2@5e4656b) fbshipit-source-id: 755ed2b297a7a6991ec10997d4491e58161348e6
Summary: Mechanically split long function into two. Reviewed By: milend Differential Revision: D38641821 fbshipit-source-id: 5f3f8718b2827816f9b98df11479785e10a01890
Summary: Code cleanup. Reviewed By: bobyangyf Differential Revision: D38681990 fbshipit-source-id: 7e44fefa6f8fcc04786fa15c45878b0dbeb70c2e
Summary: `server/mod.rs` is 2000 LOC, too long. Reviewed By: bobyangyf Differential Revision: D38682431 fbshipit-source-id: b99b07423b291cd3c80d5fcf7eefa897d9b2472e
Summary: Following diff D38712855 adds submodule. Reviewed By: bobyangyf Differential Revision: D38712856 fbshipit-source-id: 662c6d0ccb2c97cc232a813171f500678c65c95d
Summary: Following diff D38712858 adds more conversions. Reviewed By: bobyangyf Differential Revision: D38712855 fbshipit-source-id: ebc51bf7e6ad2efa5d6614ef368a4e29e2ec4acf
Reviewed By: bobyangyf Differential Revision: D38712858 fbshipit-source-id: 72e823078530ebcdb9cfc97157ebab311c23bff9
Summary: Used in the following diff D38712859. Reviewed By: bobyangyf Differential Revision: D38712854 fbshipit-source-id: 5e6269cb9979a6642bf91526d4495f649aaf4fd4
Summary: Split long file. Reviewed By: bobyangyf Differential Revision: D38682429 fbshipit-source-id: ce3b78821ea52e6c712802d21b560d0525f686f2
Summary: Split long file. Reviewed By: bobyangyf Differential Revision: D38682430 fbshipit-source-id: 16a8ce6fc51296b8c7e1b63eaa3852f31ba8c838
Summary: Make it choice between `HeapSummaryRetained` and `HeapSummaryAllocated` more explicit. Reviewed By: bobyangyf Differential Revision: D38646893 fbshipit-source-id: 8b77aa54f01f06cc28000aa2c42a1f84f9b1c023
Summary: Better make it explicit to make users to think what exactly they are profiling. Usually when profiling analysis people want retained, not allocated but: * retained is not implemented (and makes little sense) for loading profiling * later we can add an option `heap-summary` alias which could be retained for analysis or allocated for profiling Reviewed By: bobyangyf Differential Revision: D38647210 fbshipit-source-id: ff59cfd582c1bf8a2410a87f7bc35df13b1cd160
Reviewed By: milend Differential Revision: D38680952 fbshipit-source-id: e7bbbb45ca3158780ce0dde48579d26c6b02d10c
Summary: `ProfileData` later in the stack does serialization, not just `String::clone`, so it may fail. Reviewed By: milend Differential Revision: D38681026 fbshipit-source-id: d236920c817787c2c3bb1f4d2865f690ba688923
Summary: Splitting very long and unsorted `server/mod.rs`. Reviewed By: bobyangyf Differential Revision: D38682512 fbshipit-source-id: 86439bdf85b00f94f12f898b9808b6f3c7d2f8a4
Reviewed By: bobyangyf Differential Revision: D38682559 fbshipit-source-id: 70211e61e28b73891b452f19f7d105908fea13a8
Summary: Dead code. Reviewed By: bobyangyf Differential Revision: D38682608 fbshipit-source-id: afc44cd0e63c5b929dec0a0b5b0f6ddd5a981a8b
Reviewed By: bobyangyf Differential Revision: D38682641 fbshipit-source-id: 84e1e7e0c696de99a83fede733d2a24f0ce3859a
Summary: Preparation to store raw (instead of serialized) profile data in `ProfileData`. Raw mode need to be stored to be able to merge profile data (D38681490). Reviewed By: bobyangyf Differential Revision: D38681027 fbshipit-source-id: 1579ad8a1198b9812c071a4e10192cb5ad7ca15a
Summary: Refactor the SwiftDependencyInfo provider: - remove the `name` field, this is now determined from the swiftmodule filename (this has to match the module name anyway, otherwise it is a compiler error) - remove the `swiftmodule_path` field, this is now part of the new `swiftmodule_paths` tset - replace `swiftmodule_deps_paths` with a tset of artifacts to swiftmodule paths This required the following changes: - update projections to handle artifacts instead of providers - don't create deps tsets at each level, just use the ones passed up - get a deps module name from its swiftmodule path Reviewed By: maxovtsin Differential Revision: D38698477 fbshipit-source-id: f42e2a1ab72bca5f5ec4fe05c6eef7c710dbed8c
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
Note: we will need to determine where to host this follow-along repo. I'm not sure if it makes sense to sync it with fbcode as the tags need to remain in tact to make sense.
facebook-github-bot
added
the
CLA Signed
|
@arlyon has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
facebook-github-bot
force-pushed
the
main
branch
from
c53c46c to
4f0e2ec
Compare
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 6, 2022
Summary: Adds support for `tar.xz` archives to `http_archive`. The user needs to set the attribute `type = "tar.xz"` to make use of this. X-link: facebook/buck2-prelude#3 Reviewed By: arlyon Differential Revision: D40138919 Pulled By: arlyon fbshipit-source-id: 3cbecdbde522ec5fc56acee4c75cb6caf62c8e27
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 7, 2022
Summary: Pull Request resolved: facebookexperimental/allocative#3 Reviewed By: krallin Differential Revision: D41056116 Pulled By: stepancheg fbshipit-source-id: b49d2cee55c75e7dcfd6d2b4f1a9ca2229cc7bd6
facebook-github-bot
pushed a commit
that referenced
this pull request
Dec 13, 2022
Summary: Going from this stack trace: P570153247. Relevant threads are: ``` Thread 31 (LWP 16695): #0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 #1 0x0000000008761b14 in <parking_lot::raw_rwlock::RawRwLock>::lock_shared_slow () #2 0x000000000808cc3e in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::get_running_map () #3 0x00000000082c49fe in <core::future::from_generator::GenFuture<<dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::do_recompute_projection::{closure#0}> as core::future::future::Future>::poll () Thread 29 (LWP 16693): #0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 #1 0x0000000008761b14 in <parking_lot::raw_rwlock::RawRwLock>::lock_shared_slow () #2 0x000000000808a737 in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::eval_projection_task () #3 0x00000000082c4ae3 in <core::future::from_generator::GenFuture<<dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::do_recompute_projection::{closure#0}> as core::future::future::Future>::poll () Thread 19 (LWP 16683): #0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 #1 0x0000000008761b14 in <parking_lot::raw_rwlock::RawRwLock>::lock_shared_slow () #2 0x0000000008093a3b in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>> as dice::introspection::graph::EngineForIntrospection>::currently_running_key_count () Thread 15 (LWP 16679): #0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 #1 0x0000000008761b14 in <parking_lot::raw_rwlock::RawRwLock>::lock_shared_slow () #2 0x000000000808cc3e in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::get_running_map () #3 0x00000000082c49fe in <core::future::from_generator::GenFuture<<dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::do_recompute_projection::{closure#0}> as core::future::future::Future>::poll () Thread 3 (LWP 16667): #0 syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38 #1 0x0000000008762aac in <parking_lot::raw_rwlock::RawRwLock>::wait_for_readers () #2 0x0000000008760f13 in <parking_lot::raw_rwlock::RawRwLock>::lock_exclusive_slow () #3 0x000000000808cc64 in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::get_running_map () #4 0x00000000082c49fe in <core::future::from_generator::GenFuture<<dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::do_recompute_projection::{closure#0}> as core::future::future::Future>::poll () ``` The comments in the code suggest releasing an entry will release the lock, but it's not clear to me that this is actually supposed to work (the map itself is still held, you could ask it for another entry). That said, it appears somewhat pretty clear from thread 19, which is just running the introspection task, that the map is overall locked. This patch updates the code to make it extremely clear that the currently_running guard is indeed released when we call the projection task, since it's in its own scope. It's a bit unclear to me exactly where the deadlock is. I suspect it happens because `currently_running` is locked when we enter `eval_projection_task`, which means that this function cannot acquire the lock again here: ``` let sent = tx.send(node.dupe()); assert!(sent.is_ok(), "receiver is still alive"); if let Some(running_map) = self .currently_running .read() .get(&transaction_ctx.get_version()) { let removed = running_map.remove(k); assert!(removed.is_some()); } ``` That seems pretty clearly wrong, but the bit that's unclear to me is how can this ever work? Still theorizing, I suspect the reason is: Normally, it works, because: - eval_projection_versioned takes a read lock - eval_projection_task also takes a read lock But, if you ever get concurrent commands running, what will happen is: - Thread 1: eval_projection_versioned takes a read lock - Thread 2: attempts to take a write lock - Thread 1: eval_projection_task also takes a read lock, can't have it because we block new read locks when a writer is waiting. The reason I suspect I'm right is this thread, which is indeed attempting to acquire a write lock: ``` #1 0x0000000008762aac in <parking_lot::raw_rwlock::RawRwLock>::wait_for_readers () #2 0x0000000008760f13 in <parking_lot::raw_rwlock::RawRwLock>::lock_exclusive_slow () #3 0x000000000808cc64 in <dice::incremental::IncrementalEngine<dice::projection::ProjectionKeyProperties<buck2_common::legacy_configs::dice::LegacyBuckConfigCellNamesKey>>>::get_running_map () ``` Reviewed By: ndmitchell Differential Revision: D41996701 fbshipit-source-id: ba00e1e1052272ddd1a44b6c4b9d8bb924043ebb
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
facebook-github-bot
pushed a commit
that referenced
this pull request
Dec 19, 2024
…cross all host platforms Summary: ### Motivation My team has a concrete need for buck to generate 100% matching zip files for the same sets of inputs on all host platforms (macOS, Linux, Windows). Current limitations: 1. File order can be different on file system with different case sensitivity. 2. Windows can't write correct posix mode (i.e. permissions) for any entries. Although the entries themselves might fully match, those discrepancies result in different metadata, which results in a different zip file. See D67149264 for an in-depth explanation of the use case that requires this level of determinism. ### Tentative solution #1 In D66386385, I made it so the asset generation rule was only executable from Linux. Paired with buck cross builds, it made so that outputs from macOS and Linux matched, but did not work on Windows [due to some lower level buck problem](https://fb.workplace.com/groups/930797200910874/posts/1548299102494011) (still unresolved). ### Tentative solution #2 In D66404381, I wrote my own Python script to create zip files. I got all the files and metadata to match everywhere, but I could not get around differences in the compression results. Decided not to pursue because compression is important for file size. ### Tentative solution #3 In D67149264, I duplicated and tweaked buck's zip binary. It did work, but IanChilds rightfully pointed out that I'd be making maintenance on those libraries more difficult and that the team is even planning on deleting those, at some point. ### Tentative solution #4 (this diff!) IanChilds advised me to try to fix buck itself to produce consistent results, so this is me giving it a try. Because the root problem could not have been done in a backwards compatible way (the file permissions, specifically; see inlined comment), I decided to use an argument to control whether the zip tool should strive to produce a deterministic output or not, at the expense of some loss of metadata. The changes are simple and backwards compatible, but any feedback on the root problem, idea and execution are welcome. Reviewed By: christolliday Differential Revision: D67301945 fbshipit-source-id: c42ef7a52efd235b43509337913d905bcbaf3782
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.