Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Commit

Permalink
feat: Update boojum nightly - feature gate packed simd (Attempt 2) (#50)
Browse files Browse the repository at this point in the history
  • Loading branch information
mm-zk authored May 29, 2024
1 parent d402c88 commit 4bcb11f
Show file tree
Hide file tree
Showing 16 changed files with 1,020 additions and 92 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,25 @@ jobs:
- uses: actions-rust-lang/setup-rust-toolchain@v1
- run: cargo build --verbose
- run: cargo test --verbose --all

build_old:
name: cargo build and test (packed_simd)
strategy:
matrix:
# Needs big runners to run tests
# Only macos-13-xlarge is Apple Silicon, as per:
# https://docs.github.com/en/actions/using-github-hosted-runners/about-larger-runners/about-larger-runners#about-macos-larger-runners
os: [ubuntu-22.04-github-hosted-16core, macos-13-xlarge]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: nightly-2023-05-31

# Still compile the old rust nightly with packed simd - until we have a good replacement in poseidon.
- run: RUSTFLAGS=-Awarnings cargo +nightly-2023-05-31 build --features include_packed_simd
- run: RUSTFLAGS=-Awarnings cargo +nightly-2023-05-31 test --features include_packed_simd

formatting:
name: cargo fmt
Expand Down
8 changes: 7 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ itertools = "0.10"
blake2 = "0.10"
sha2 = "0.10"
num-modular = "0.5.1"
packed_simd = { version = "0.3.9" }
packed_simd = { version = "0.3.9" , optional = true}
pairing = { package = "pairing_ce", git = "https://github.com/matter-labs/pairing.git" }
crypto-bigint = "0.5"
convert_case = "*"
Expand Down Expand Up @@ -54,3 +54,9 @@ opt-level = 3
[features]
# If enabled, logs will be using trace, if disabled, they will be printed to stdout.
log_tracing = ["tracing"]
# Currently packed_simd is no longer working with the newest nightly.
# But we still keep it as a feature, as we didn't migrate all the code, and
# some people might want to use older rust nightly, to be able to gain some performance.
include_packed_simd = ["packed_simd"]
cr_paranoia_mode = []
debug_track = []
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[toolchain]
channel = "nightly-2023-06-25"
channel = "nightly-2024-05-07"
2 changes: 1 addition & 1 deletion src/cs/traits/cs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl<'set, 'tgt: 'set, T: SmallField> DstBuffer<'set, 'tgt, T> {
*offset += 1;
}
DstBuffer::MutSliceIndirect(dst, debug_track, offset) => {
if cfg!(debug_track) && *debug_track {
if cfg!(feature = "debug_track") && *debug_track {
log!(" set out {} <- {}", *offset, value.as_raw_u64())
}

Expand Down
4 changes: 2 additions & 2 deletions src/dag/guide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ impl<'a, T: Copy + Debug, F: SmallField, Cfg: CSResolverConfig> GuideOrder<'a, T
pos += span.buffer.len();
}

if cfg!(cr_paranoia_mode) && self.guide.tracing {
if cfg!(feature = "cr_paranoia_mode") && self.guide.tracing {
log!(
"Released span {}: {:?}",
self.guide.spans[0].id.0,
Expand Down Expand Up @@ -684,7 +684,7 @@ impl<T: Debug, F: SmallField, Cfg: CSResolverConfig> BufferGuide<T, F, Cfg> {
}

pub(crate) fn flush(&mut self) -> BufferGuideFinalization<'_, T, F, Cfg> {
if cfg!(cr_paranoia_mode) && self.tracing {
if cfg!(feature = "cr_paranoia_mode") && self.tracing {
log!("CRG: flush.");
}

Expand Down
12 changes: 9 additions & 3 deletions src/dag/resolver_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ pub(crate) fn invocation_binder<Fn, F: SmallField>(
// Safety: This is the actual type of the provided function.
let bound = resolver.resolve_fn::<Fn>();

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && false {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA) && false {
log!(
"Ivk: Ins [{}], Out [{}], Out-addr [{}], Thread [{}]",
resolver
Expand All @@ -448,7 +448,10 @@ pub(crate) fn invocation_binder<Fn, F: SmallField>(
)
}

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && debug_track && false {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA)
&& debug_track
&& false
{
log!(
"Ivk: provided inputs:\n - {:?}",
ins.iter().map(|x| x.as_raw_u64()).collect_vec()
Expand All @@ -457,7 +460,10 @@ pub(crate) fn invocation_binder<Fn, F: SmallField>(

bound(ins, &mut DstBuffer::MutSliceIndirect(out, debug_track, 0));

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && debug_track && true {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA)
&& debug_track
&& true
{
log!(
"Ivk: calculated outputs:\n - {:?}",
out.iter().map(|x| x.as_raw_u64()).collect_vec()
Expand Down
12 changes: 6 additions & 6 deletions src/dag/resolvers/mt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ impl<V: SmallField, RS: ResolverSortingMode<V>, CFG: CSResolverConfig>

let debug_track = vec![];

if cfg!(cr_paranoia_mode) || PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || PARANOIA {
log!("Contains tracked keys {:?} ", debug_track);
}

Expand Down Expand Up @@ -269,7 +269,7 @@ impl<V: SmallField, RS: ResolverSortingMode<V>, CFG: CSResolverConfig>

self.sorter.write_sequence();

if cfg!(cr_paranoia_mode) || PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || PARANOIA {
log!("CR {:?}", unsafe {
self.common.awaiters_broker.stats.u_deref()
});
Expand Down Expand Up @@ -1487,7 +1487,7 @@ mod test {

storage.wait_till_resolved();

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
log!("Test: total value result: \n - {}", unsafe {
(*storage.common.values.get())
.variables
Expand All @@ -1509,7 +1509,7 @@ mod test {
let act = Place::from_variable(Variable::from_variable_index(ix as u64))
.to(|x| storage.get_value_unchecked(x));

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
log!("Test: per item value: ix {}, value {}", ix, act);
}

Expand Down Expand Up @@ -1542,7 +1542,7 @@ mod test {

storage.wait_till_resolved();

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
log!("Test: total value result: \n - {}", unsafe {
(*storage.common.values.get())
.variables
Expand All @@ -1564,7 +1564,7 @@ mod test {
let act = Place::from_variable(Variable::from_variable_index(ix as u64))
.to(|x| storage.get_value_unchecked(x));

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
log!("Test: per item value: ix {}, value {}", ix, act);
}

Expand Down
2 changes: 1 addition & 1 deletion src/dag/resolvers/mt/registrar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl Registrar {
}

pub(crate) fn is_empty(&self) -> bool {
if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
log!(
"CRR: total remaining resolvers: {}",
self.vars.values().map(|x| x.len()).sum::<usize>()
Expand Down
34 changes: 19 additions & 15 deletions src/dag/resolvers/mt/resolution_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,12 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>
comms,

track_list: Vec::new(),
execution_list: if cfg!(cr_paranoia_mode) { 1 << 26 } else { 0 }
.to(|x| Vec::with_capacity(x).op(|v| v.resize(x, 0))),
execution_list: if cfg!(feature = "cr_paranoia_mode") {
1 << 26
} else {
0
}
.to(|x| Vec::with_capacity(x).op(|v| v.resize(x, 0))),
phantom: PhantomData,
};

Expand Down Expand Up @@ -207,7 +211,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>

data[data_ix].push(order_ix.into(), task.order_info.value);

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
self.execution_list[order_ix] += 1;

if self.execution_list[order_ix] > 1 {
Expand Down Expand Up @@ -238,7 +242,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>
}
}

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && true {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA) && true {
log!("RW: Batch! {} tasks.", count);
}

Expand All @@ -264,7 +268,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>
.for_each(|x| {
x.state = ResolverState::Done;

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
unsafe {
let r = self.common.resolvers.u_deref().get(x.order_info.value);

Expand All @@ -291,7 +295,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>
}
});

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
if self
.exec_order_buffer
.iter()
Expand Down Expand Up @@ -343,7 +347,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>

drop(awaiters);

if cfg!(cr_paranoia_mode) && count > 0 {
if cfg!(feature = "cr_paranoia_mode") && count > 0 {
log!(
"RW: Shifted by {}, new range is: {}..{}, buffer len: {}",
count,
Expand Down Expand Up @@ -412,7 +416,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>

self.stats.total_consumption = extend_to as u64;

if crate::dag::resolvers::mt::PARANOIA || cfg!(cr_paranoia_mode) {
if crate::dag::resolvers::mt::PARANOIA || cfg!(feature = "cr_paranoia_mode") {
log!(
"RW: Extended range by {}, new range {}..{}",
extend_to,
Expand Down Expand Up @@ -474,7 +478,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>
}
}

if crate::dag::resolvers::mt::PARANOIA || cfg!(cr_paranoia_mode) {
if crate::dag::resolvers::mt::PARANOIA || cfg!(feature = "cr_paranoia_mode") {
log!("[{:?}] RW: Exit conditions met.", std::time::Instant::now())
}

Expand All @@ -484,7 +488,7 @@ impl<V: SmallField + 'static, T: TrackId + 'static, Cfg: RWConfig<T> + 'static>

self.stats.total_time = start_instant.elapsed();

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
log!("CR {:#?}", self.stats);
log!("CR {:#?}", unsafe { &*self.channel.stats.get() });

Expand Down Expand Up @@ -554,7 +558,7 @@ impl<V: SmallField, T: TrackId + 'static, Cfg: RWConfig<T>, const SIZE: usize>
// here, as this is an unsynchronizd access.
let resolver = this.common.resolvers.u_deref().get(*resolver_ix);

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature="cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
this.invoke(resolver, *order_ix);

Expand Down Expand Up @@ -590,7 +594,7 @@ impl<V: SmallField, T: TrackId + 'static, Cfg: RWConfig<T>, const SIZE: usize>
});
}

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
log!(
"{}\n{:#?}\n{:#?}",
std::thread::current().name().unwrap_or_default(),
Expand Down Expand Up @@ -629,7 +633,7 @@ impl<V: SmallField, T: TrackId + 'static, Cfg: RWConfig<T>, const SIZE: usize>
.map(|x| {
let (vs, md) = self.common.values.u_deref().get_item_ref(*x);

if cfg!(cr_paranoia_mode) || true {
if cfg!(feature = "cr_paranoia_mode") || true {
if Cfg::ASSERT_TRACKED_VALUES {
assert!(md.is_tracked());
}
Expand Down Expand Up @@ -678,7 +682,7 @@ impl<V: SmallField, T: TrackId + 'static, Cfg: RWConfig<T>, const SIZE: usize>

let mut track = false;

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
if let Some(x) = self
.debug_track
.iter()
Expand Down Expand Up @@ -831,7 +835,7 @@ impl LockStepChannel {
fn execute(&self) {
use std::sync::atomic::Ordering::*;

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && false {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA) && false {
log!("RW: batch sent {:#?}", unsafe { self.data.u_deref() });
}

Expand Down
14 changes: 7 additions & 7 deletions src/dag/resolvers/mt/sorters/sorter_live.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter>
}
}

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
// This ugly block checks that the calculated parallelism is
// correct. It's a bit slower than O(n^2). Also note, that it
// checks only the last 1050 items, so it's not a full check,
Expand Down Expand Up @@ -297,7 +297,7 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter> ResolverS
}

fn set_value(&mut self, key: crate::cs::Place, value: F) {
if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA)
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA)
&& self.debug_track.contains(&key)
&& false
{
Expand Down Expand Up @@ -378,7 +378,7 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter> ResolverS

let mut hit = false;

if (cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA) && true {
if (cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA) && true {
if let Some(x) = self.debug_track.iter().find(|x| inputs.contains(x)) {
log!("CR: added resolution with tracked input {:?}", x);

Expand Down Expand Up @@ -498,7 +498,7 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter> ResolverS
outputs: &[Place],
added_at: RegistrationNum,
) -> Vec<ResolverIx> {
if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
if let Some(x) = self.debug_track.iter().find(|x| inputs.contains(x)) {
log!("CR: internalized resolution with tracked input {:?}", x);
}
Expand All @@ -519,7 +519,7 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter> ResolverS

let deps = inputs.iter().map(|x| &values.get_item_ref(*x).1);

if cfg!(cr_paranoia_mode) {
if cfg!(feature = "cr_paranoia_mode") {
debug_assert!(
deps.clone().all(|x| { x.is_tracked() }),
"Attempting to internalize a resolution with an untracked input. All inputs must be tracked."
Expand Down Expand Up @@ -610,14 +610,14 @@ impl<F: SmallField, Cfg: CSResolverConfig, RW: ResolutionRecordWriter> ResolverS
self.record.values_count = unsafe { self.common.values.u_deref().max_tracked + 1 } as usize;
self.record.registrations_count = self.stats.registrations_added as usize;

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
log!(
"CR: Final order written. Order len {}",
self.common.exec_order.lock().unwrap().items.len()
);
}

if cfg!(cr_paranoia_mode) || crate::dag::resolvers::mt::PARANOIA {
if cfg!(feature = "cr_paranoia_mode") || crate::dag::resolvers::mt::PARANOIA {
self.guide.stats.finalize();

log!("CR {:?}", self.guide.stats);
Expand Down
Loading

0 comments on commit 4bcb11f

Please sign in to comment.