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

feat: Update boojum nightly - feature gate packed simd (Attempt 2) #50

Merged
merged 10 commits into from
May 29, 2024
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
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
Loading