Skip to content

Use hashbrown replacements for std equivalents #947

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

Merged
merged 1 commit into from
Jun 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,34 @@ jobs:
# 1.41.0 is Debian stable
1.41.0,
# 1.45.2 is MSRV for lightning-net-tokio, lightning-block-sync, and coverage generation
1.45.2]
1.45.2,
# 1.49.0 is MSRV for no_std builds using hashbrown
1.49.0]
include:
- toolchain: stable
build-net-tokio: true
build-no-std: true
- toolchain: stable
platform: macos-latest
build-net-tokio: true
build-no-std: true
- toolchain: stable
platform: windows-latest
build-net-tokio: true
build-no-std: true
- toolchain: beta
build-net-tokio: true
build-no-std: true
- toolchain: 1.36.0
build-no-std: false
- toolchain: 1.41.0
build-no-std: false
- toolchain: 1.45.2
build-net-tokio: true
build-no-std: false
coverage: true
- toolchain: 1.49.0
build-no-std: true
runs-on: ${{ matrix.platform }}
steps:
- name: Checkout source code
Expand All @@ -47,7 +60,10 @@ jobs:
run: RUSTFLAGS="-C link-dead-code" cargo build --verbose --color always
- name: Build on Rust ${{ matrix.toolchain }}
if: "! matrix.build-net-tokio"
run: cargo build --verbose --color always -p lightning && cargo build --verbose --color always -p lightning-invoice && cargo build --verbose --color always -p lightning-persister
run: |
cargo build --verbose --color always -p lightning
cargo build --verbose --color always -p lightning-invoice
cargo build --verbose --color always -p lightning-persister
- name: Build Block Sync Clients on Rust ${{ matrix.toolchain }} with features
if: "matrix.build-net-tokio && !matrix.coverage"
run: |
Expand All @@ -56,7 +72,6 @@ jobs:
cargo build --verbose --color always --features rpc-client
cargo build --verbose --color always --features rpc-client,rest-client
cargo build --verbose --color always --features rpc-client,rest-client,tokio
cd ..
- name: Build Block Sync Clients on Rust ${{ matrix.toolchain }} with features and full code-linking for coverage generation
if: matrix.coverage
run: |
Expand All @@ -65,16 +80,30 @@ jobs:
RUSTFLAGS="-C link-dead-code" cargo build --verbose --color always --features rpc-client
RUSTFLAGS="-C link-dead-code" cargo build --verbose --color always --features rpc-client,rest-client
RUSTFLAGS="-C link-dead-code" cargo build --verbose --color always --features rpc-client,rest-client,tokio
cd ..
- name: Test on Rust ${{ matrix.toolchain }} with net-tokio
if: "matrix.build-net-tokio && !matrix.coverage"
run: cargo test --verbose --color always
- name: Test on Rust ${{ matrix.toolchain }} with net-tokio and full code-linking for coverage generation
if: matrix.coverage
run: RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always
- name: Test on no_std bullds Rust ${{ matrix.toolchain }}
if: "matrix.build-no-std && !matrix.coverage"
run: |
cd lightning
cargo test --verbose --color always --features hashbrown
cd ..
- name: Test on no_std bullds Rust ${{ matrix.toolchain }} and full code-linking for coverage generation
if: "matrix.build-no-std && matrix.coverage"
run: |
cd lightning
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --features hashbrown
cd ..
- name: Test on Rust ${{ matrix.toolchain }}
if: "! matrix.build-net-tokio"
run: cargo test --verbose --color always -p lightning && cargo test --verbose --color always -p lightning-invoice && cargo build --verbose --color always -p lightning-persister
run: |
cargo test --verbose --color always -p lightning
cargo test --verbose --color always -p lightning-invoice
cargo build --verbose --color always -p lightning-persister
- name: Test Block Sync Clients on Rust ${{ matrix.toolchain }} with features
if: "matrix.build-net-tokio && !matrix.coverage"
run: |
Expand All @@ -83,7 +112,6 @@ jobs:
cargo test --verbose --color always --features rpc-client
cargo test --verbose --color always --features rpc-client,rest-client
cargo test --verbose --color always --features rpc-client,rest-client,tokio
cd ..
- name: Test Block Sync Clients on Rust ${{ matrix.toolchain }} with features and full code-linking for coverage generation
if: matrix.coverage
run: |
Expand All @@ -92,7 +120,6 @@ jobs:
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --features rpc-client
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --features rpc-client,rest-client
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --features rpc-client,rest-client,tokio
cd ..
- name: Install deps for kcov
if: matrix.coverage
run: |
Expand Down Expand Up @@ -157,6 +184,7 @@ jobs:
run: |
cd lightning
RUSTFLAGS="--cfg=require_route_graph_test" cargo test
RUSTFLAGS="--cfg=require_route_graph_test" cargo test --features hashbrown
cd ..
- name: Run benchmarks on Rust ${{ matrix.toolchain }}
run: |
Expand Down
1 change: 1 addition & 0 deletions ci/check-compiles.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ cargo check
cargo doc
cargo doc --document-private-items
cd fuzz && cargo check --features=stdin_fuzz
cd ../lightning && cargo check --no-default-features --features=no_std
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In .github/workflows/build.yml can you also add a cd lightning && cargo test --verbose --color always --features hashbrown to run the test suite with hashbrown + std?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which builds should that run under, or do you want a new build target?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhhmmmmm, yes? I don't think it matters too much, maybe in the existing ones is simplest, given it's ~one line?

2 changes: 2 additions & 0 deletions lightning/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ max_level_debug = []
# This is unsafe to use in production because it may result in the counterparty publishing taking our funds.
unsafe_revoked_tx_signing = []
unstable = []
no_std = ["hashbrown"]

[dependencies]
bitcoin = "0.26"

hashbrown = { version = "0.11", optional = true }
hex = { version = "0.3", optional = true }
regex = { version = "0.1.80", optional = true }

Expand Down
1 change: 0 additions & 1 deletion lightning/src/chain/chainmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use util::events;
use util::events::EventHandler;

use prelude::*;
use std::collections::{HashMap, hash_map};
use std::sync::RwLock;
use core::ops::Deref;

Expand Down
1 change: 0 additions & 1 deletion lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ use util::byte_utils;
use util::events::Event;

use prelude::*;
use std::collections::{HashMap, HashSet};
use core::{cmp, mem};
use std::io::Error;
use core::ops::Deref;
Expand Down
1 change: 0 additions & 1 deletion lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelP
use ln::msgs::UnsignedChannelAnnouncement;

use prelude::*;
use std::collections::HashSet;
use core::sync::atomic::{AtomicUsize, Ordering};
use std::io::Error;
use ln::msgs::{DecodeError, MAX_VALUE_MSAT};
Expand Down
1 change: 0 additions & 1 deletion lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use util::byte_utils;

use prelude::*;
use alloc::collections::BTreeMap;
use std::collections::HashMap;
use core::cmp;
use core::ops::Deref;
use core::mem::replace;
Expand Down
11 changes: 9 additions & 2 deletions lightning/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,12 @@ pub mod ln;
pub mod routing;

mod prelude {
pub use alloc::{vec, vec::Vec, string::String};
}
#[cfg(feature = "hashbrown")]
extern crate hashbrown;

pub use alloc::{vec, vec::Vec, string::String, collections::VecDeque};
#[cfg(not(feature = "hashbrown"))]
pub use std::collections::{HashMap, HashSet, hash_map};
#[cfg(feature = "hashbrown")]
pub use self::hashbrown::{HashMap, HashSet, hash_map};
}
1 change: 0 additions & 1 deletion lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ use ln::functional_test_utils::*;
use util::test_utils;

use prelude::*;
use std::collections::HashMap;
use std::sync::{Arc, Mutex};

// If persister_fail is true, we have the persister return a PermanentFailure
Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ use util::errors::APIError;
use prelude::*;
use core::{cmp, mem};
use core::cell::RefCell;
use std::collections::{HashMap, hash_map, HashSet};
use std::io::{Cursor, Read};
use std::sync::{Arc, Condvar, Mutex, MutexGuard, RwLock, RwLockReadGuard};
use core::sync::atomic::{AtomicUsize, Ordering};
Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ use core::cell::RefCell;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
use core::mem;
use std::collections::HashMap;

pub const CHAN_CONFIRM_DEPTH: u32 = 10;

Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ use regex;

use prelude::*;
use alloc::collections::BTreeSet;
use std::collections::{HashMap, HashSet};
use core::default::Default;
use std::sync::{Arc, Mutex};

Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/peer_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use routing::network_graph::NetGraphMsgHandler;

use prelude::*;
use alloc::collections::LinkedList;
use std::collections::{HashMap,hash_map,HashSet};
use std::sync::{Arc, Mutex};
use core::sync::atomic::{AtomicUsize, Ordering};
use core::{cmp, hash, fmt, mem};
Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/reorg_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::hash_types::BlockHash;

use prelude::*;
use std::collections::HashMap;
use core::mem;

use ln::functional_test_utils::*;
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/routing/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use util::logger::Logger;
use prelude::*;
use alloc::collections::BinaryHeap;
use core::cmp;
use std::collections::HashMap;
use core::ops::Deref;

/// A hop in a route
Expand Down Expand Up @@ -3843,16 +3842,19 @@ mod tests {
}
}

#[cfg(not(feature = "no_std"))]
pub(super) fn random_init_seed() -> u64 {
// Because the default HashMap in std pulls OS randomness, we can use it as a (bad) RNG.
use core::hash::{BuildHasher, Hasher};
let seed = std::collections::hash_map::RandomState::new().build_hasher().finish();
println!("Using seed of {}", seed);
seed
}
#[cfg(not(feature = "no_std"))]
use util::ser::Readable;

#[test]
#[cfg(not(feature = "no_std"))]
fn generate_routes() {
let mut d = match super::test_utils::get_route_file() {
Ok(f) => f,
Expand Down Expand Up @@ -3880,6 +3882,7 @@ mod tests {
}

#[test]
#[cfg(not(feature = "no_std"))]
fn generate_routes_mpp() {
let mut d = match super::test_utils::get_route_file() {
Ok(f) => f,
Expand Down Expand Up @@ -3907,7 +3910,7 @@ mod tests {
}
}

#[cfg(test)]
#[cfg(all(test, not(feature = "no_std")))]
pub(crate) mod test_utils {
use std::fs::File;
/// Tries to open a network graph file, or panics with a URL to fetch it.
Expand All @@ -3934,7 +3937,7 @@ pub(crate) mod test_utils {
}
}

#[cfg(all(test, feature = "unstable"))]
#[cfg(all(test, feature = "unstable", not(feature = "no_std")))]
mod benches {
use super::*;
use util::logger::{Logger, Record};
Expand Down
1 change: 0 additions & 1 deletion lightning/src/util/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

use prelude::*;
use std::io::{Read, Write};
use std::collections::HashMap;
use core::hash::Hash;
use std::sync::Mutex;
use core::cmp;
Expand Down
1 change: 0 additions & 1 deletion lightning/src/util/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ use core::time::Duration;
use std::sync::{Mutex, Arc};
use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use core::{cmp, mem};
use std::collections::{HashMap, HashSet, VecDeque};
use chain::keysinterface::InMemorySigner;

pub struct TestVecWriter(pub Vec<u8>);
Expand Down