Skip to content

Commit

Permalink
enable debug assertions in CI (#10205)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ekleog-NEAR authored Nov 20, 2023
1 parent 7934b34 commit fdead43
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 46 deletions.
14 changes: 7 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ jobs:
prefix-key: "0" # change this to invalidate CI cache
shared-key: "cargo_nextest-linux"
save-if: "false" # use the cache from nextest, but don’t double-save
- run: cargo build --locked --profile quick-release -p neard --bin neard
- run: cargo build --locked --profile dev-release -p neard --bin neard
- uses: actions/upload-artifact@v3
with:
name: neard
path: target/quick-release/neard
path: target/dev-release/neard
if-no-files-found: error
retention-days: 1

Expand Down Expand Up @@ -61,7 +61,7 @@ jobs:
with:
prefix-key: "0" # change this to invalidate CI cache
shared-key: "cargo_nextest-${{ matrix.cache_id }}"
- run: cargo nextest run --locked --workspace -p '*' --cargo-profile quick-release --profile ci ${{ matrix.flags }}
- run: cargo nextest run --locked --workspace -p '*' --cargo-profile dev-release --profile ci ${{ matrix.flags }}
env:
RUST_BACKTRACE: short

Expand Down Expand Up @@ -140,13 +140,13 @@ jobs:
- run: pip3 install --user -r pytest/requirements.txt
# This is the only job that uses `--features nightly` so we build this in-line instead of a
# separate job like done with the regular neard.
- run: cargo build --profile quick-release -p neard --bin neard --features nightly
- run: cargo build --profile dev-release -p neard --bin neard --features nightly
# Note: We're not running spin_up_cluster.py for non-nightly
# because spinning up non-nightly clusters is already covered
# by other steps in the CI, e.g. upgradable.
- run: python3 pytest/tests/sanity/spin_up_cluster.py
env:
NEAR_ROOT: "target/quick-release"
NEAR_ROOT: "target/dev-release"

py_genesis_check:
name: "Genesis Changes"
Expand All @@ -161,8 +161,8 @@ jobs:
- uses: actions/download-artifact@v3
with:
name: neard
path: target/quick-release
- run: echo "CURRENT_NEARD=$PWD/target/quick-release/neard" >> "$GITHUB_ENV"
path: target/dev-release
- run: echo "CURRENT_NEARD=$PWD/target/dev-release/neard" >> "$GITHUB_ENV"
- run: chmod +x "$CURRENT_NEARD"
- run: pip3 install --user -r pytest/requirements.txt
- run: python3 scripts/state/update_res.py check
Expand Down
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,12 @@ panic = 'abort'
lto = "fat"
codegen-units = 1

# A much faster to compile version of `release`.
[profile.quick-release]
# A much faster to compile version of `release`, for development use.
[profile.dev-release]
inherits = "release"
lto = false
codegen-units = 16
debug-assertions = true

# Used for fuzzing, LTO is ill-supported as of 2023-09 and so should not be enabled.
[profile.fuzz]
Expand Down
34 changes: 17 additions & 17 deletions core/store/src/trie/prefetching_trie_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,18 +322,18 @@ impl PrefetchStagingArea {
/// 3: Main thread value is inserted in shard cache.
pub(crate) fn release(&self, key: &CryptoHash) {
let mut guard = self.0.lock_mut();
let dropped = guard.slots.remove(key);
let _dropped = guard.slots.remove(key);
// `Done` is the result after a successful prefetch.
// `PendingFetch` means the value has been read without a prefetch.
// `None` means prefetching was stopped due to memory limits.
debug_assert!(
dropped.is_none()
|| prefetch_state_matches(
PrefetchSlot::Done(Arc::new([])),
dropped.as_ref().unwrap()
)
|| prefetch_state_matches(PrefetchSlot::PendingFetch, dropped.as_ref().unwrap()),
);
// debug_assert!(
// dropped.is_none()
// || prefetch_state_matches(
// PrefetchSlot::Done(Arc::new([])),
// dropped.as_ref().unwrap()
// )
// || prefetch_state_matches(PrefetchSlot::PendingFetch, dropped.as_ref().unwrap()),
// );
}

/// Block until value is prefetched and then return it.
Expand Down Expand Up @@ -504,14 +504,14 @@ impl PrefetchApi {
}
}

fn prefetch_state_matches(expected: PrefetchSlot, actual: &PrefetchSlot) -> bool {
match (expected, actual) {
(PrefetchSlot::PendingPrefetch, PrefetchSlot::PendingPrefetch)
| (PrefetchSlot::PendingFetch, PrefetchSlot::PendingFetch)
| (PrefetchSlot::Done(_), PrefetchSlot::Done(_)) => true,
_ => false,
}
}
// fn prefetch_state_matches(expected: PrefetchSlot, actual: &PrefetchSlot) -> bool {
// match (expected, actual) {
// (PrefetchSlot::PendingPrefetch, PrefetchSlot::PendingPrefetch)
// | (PrefetchSlot::PendingFetch, PrefetchSlot::PendingFetch)
// | (PrefetchSlot::Done(_), PrefetchSlot::Done(_)) => true,
// _ => false,
// }
// }

/// Guard that owns the spawned prefetching IO threads.
#[must_use = "When dropping this handle, the IO threads will be aborted immediately."]
Expand Down
8 changes: 4 additions & 4 deletions docs/practices/fast_builds.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ lto (link-time optimization), so our `-r` builds are very slow, use a lot of
RAM, and don't utilize the available parallelism fully.

As debug builds are much too slow at runtime for many purposes, we have a custom
profile `--profile quick-release` which is equivalent to `-r`, except that the
time-consuming options such as LTO are disabled.
profile `--profile dev-release` which is equivalent to `-r`, except that the
time-consuming options such as LTO are disabled, and debug assertions are enabled.

Use `--profile quick-release` when doing comparative benchmarking, or when
connecting a locally built node to a network. Use `-r` if you want to get
Use `--profile dev-release` for most local development, or when connecting a
locally built node to a network. Use `-r` for production, or if you want to get
absolute performance numbers.

## Linker
Expand Down
4 changes: 2 additions & 2 deletions docs/practices/workflows/deploy_a_contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ deploy a contract. You might want to re-read [how to run a node](./run_a_node.md
to understand what's going one here:

```console
$ cargo run --profile quick-release -p neard -- init
$ cargo run --profile quick-release -p neard -- run
$ cargo run --profile dev-release -p neard -- init
$ cargo run --profile dev-release -p neard -- run
$ NEAR_ENV=local near create-account alice.test.near --masterAccount test.near
```

Expand Down
4 changes: 2 additions & 2 deletions docs/practices/workflows/io_trace.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ Run the following command to see an overview of available commands.

```bash
# will print the help page for the IO trace replay command
cargo run --profile quick-release -p runtime-params-estimator -- \
cargo run --profile dev-release -p runtime-params-estimator -- \
replay --help
```

Expand All @@ -281,7 +281,7 @@ columns that were accessed and how many times, aggregated by chunk.


```bash
cargo run --profile quick-release -p runtime-params-estimator -- \
cargo run --profile dev-release -p runtime-params-estimator -- \
replay ./path/to/my.io_trace chunk-db-stats
```

Expand Down
18 changes: 9 additions & 9 deletions docs/practices/workflows/run_a_node.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ relatively little attention to the various shortcuts we have.
Start with the following command:

```console
$ cargo run --profile quick-release -p neard -- --help
$ cargo run --profile dev-release -p neard -- --help
```

This command builds `neard` and asks it to show `--help`. Building `neard` takes
Expand All @@ -20,13 +20,13 @@ Let's dissect the command:

- `cargo run` asks `Cargo`, the package manager/build tool, to run our
application. If you don't have `cargo`, install it via <https://rustup.rs>
- `--profile quick-release` is our
- `--profile dev-release` is our
[custom profile](https://doc.rust-lang.org/cargo/reference/profiles.html#custom-profiles)
to build a somewhat optimized version of the code. The default debug
profile is faster to compile, but produces a node that is too slow to
participate in a real network. The `--release` profile produces a fully
optimized node, but that's very slow to compile. So `--quick-release`
is a sweet spot for us!
optimized node, but that's very slow to compile. So `--dev-release`
is a sweet spot for us! However, never use it for actual production nodes.
- `-p neard` asks to build the `neard` package. We use
[cargo workspaces](https://doc.rust-lang.org/cargo/reference/workspaces.html)
to organize our code. The `neard` package in the top-level `/neard` directory
Expand All @@ -49,7 +49,7 @@ The first step there is creating the required configuration. Run the `init`
command to create config files:

```console
$ cargo run --profile quick-release -p neard -- init
$ cargo run --profile dev-release -p neard -- init
INFO neard: version="trunk" build="1.1.0-3091-ga8964d200-modified" latest_protocol=57
INFO near: Using key ed25519:B41GMfqE2jWHVwrPLbD7YmjZxxeQE9WA9Ua2jffP5dVQ for test.near
INFO near: Using key ed25519:34d4aFJEmc2A96UXMa9kQCF8g2EfzZG9gCkBAPcsVZaz for node
Expand Down Expand Up @@ -193,7 +193,7 @@ is rejected.
Finally,

```console
$ cargo run --profile quick-release -p neard -- run
$ cargo run --profile dev-release -p neard -- run
INFO neard: version="trunk" build="1.1.0-3091-ga8964d200-modified" latest_protocol=57
INFO near: Creating a new RocksDB database path=/home/matklad/.near/data
INFO db: Created a new RocksDB instance. num_instances=1
Expand Down Expand Up @@ -246,7 +246,7 @@ here. The important bit here is that the node remembers the state of the network
so, when we restart it, it continues from around the last block:

```console
$ cargo run --profile quick-release -p neard -- run
$ cargo run --profile dev-release -p neard -- run
INFO neard: version="trunk" build="1.1.0-3091-ga8964d200-modified" latest_protocol=57
INFO db: Created a new RocksDB instance. num_instances=1
INFO db: Dropped a RocksDB instance. num_instances=0
Expand Down Expand Up @@ -467,8 +467,8 @@ some amount of tokens was deducted to account for transaction fees.
Great! So we've learned how to run our very own single-node NEAR network using a
binary we've built from source. The steps are:

- Create configs with `cargo run --profile quick-release -p neard -- init`
- Run the node with `cargo run --profile quick-release -p neard -- run`
- Create configs with `cargo run --profile dev-release -p neard -- init`
- Run the node with `cargo run --profile dev-release -p neard -- run`
- Poke the node with `httpie` or
- Install `near-cli` via `npm install -g near-cli`
- Submit transactions via `NEAR_ENV=local near create-account ...`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl Mode {
fn cargo_profile(self) -> &'static str {
match self {
Mode::Default => "release",
Mode::Fast => "quick-release",
Mode::Fast => "dev-release",
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ mod tests {
/// Run a minimal estimation of all parameters to ensure we have no crashes.
///
/// Things to note:
/// - This re-compiles the estimator (including nearcore) in quick-release
/// - This re-compiles the estimator (including nearcore) in dev-release
/// profile because estimations are really slow otherwise.
/// - This is an expensive test. We run it like any other test for now but
/// it might make sense to put it in a separate CI job.
Expand Down
2 changes: 1 addition & 1 deletion runtime/runtime-params-estimator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ fn main_docker(
json_output: bool,
debug: bool,
) -> anyhow::Result<()> {
let profile = if full { "release" } else { "quick-release" };
let profile = if full { "release" } else { "dev-release" };
exec("docker --version").context("please install `docker`")?;

let project_root = project_root();
Expand Down

0 comments on commit fdead43

Please sign in to comment.