Skip to content
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

Bump rust-toolchain to 1.59.0 #1607

Merged
merged 3 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 0 additions & 2 deletions fil-proofs-param/src/bin/fakeipfsadd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ enum Cli {
Add {
#[structopt(help = "Positional argument for the path to the file to add.")]
file_path: String,
#[structopt(short = "Q", help = "Simulates the -Q argument to `ipfs add`.")]
vmx marked this conversation as resolved.
Show resolved Hide resolved
quieter: bool,
},
}

Expand Down
2 changes: 1 addition & 1 deletion fil-proofs-param/src/bin/parampublish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fn get_filenames_in_cache_dir() -> Vec<String> {
fn publish_file(ipfs_bin: &str, filename: &str) -> Result<String> {
let path = get_full_path_for_file_within_cache(filename);
let output = Command::new(ipfs_bin)
.args(&["add", "-Q", path.to_str().unwrap()])
.args(&["add", path.to_str().unwrap()])
.output()
.expect("failed to run ipfs subprocess");
stderr()
Expand Down
10 changes: 1 addition & 9 deletions fil-proofs-param/src/bin/srspublish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ fn get_filenames_in_cache_dir() -> Vec<String> {
fn publish_file(ipfs_bin: &str, filename: &str) -> Result<String> {
let path = get_full_path_for_file_within_cache(filename);
let output = Command::new(ipfs_bin)
.args(&["add", "-Q", path.to_str().unwrap()])
.args(&["add", path.to_str().unwrap()])
.output()
.expect("failed to run ipfs subprocess");
stderr()
Expand All @@ -102,14 +102,6 @@ fn write_param_map_to_disk(param_map: &ParameterMap, json_path: &str) -> Result<
#[derive(Debug, StructOpt)]
#[structopt(name = "srspublish", version = "1.0", about = CLI_ABOUT.as_str())]
struct Cli {
#[structopt(
long = "list-all",
short = "a",
help = "The user will be prompted to select the files to publish from the set of all files \
found in the cache dir. Excluding the -a/--list-all flag will result in the user being \
prompted for a single param version number for filtering-in files in the cache dir."
)]
list_all_files: bool,
#[structopt(
long = "ipfs-bin",
value_name = "PATH TO IPFS BINARY",
Expand Down
71 changes: 38 additions & 33 deletions fil-proofs-param/tests/paramfetch/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,42 +63,47 @@ impl ParamFetchSessionBuilder {

/// Launch paramfetch in an environment configured by the builder.
pub fn build(self) -> ParamFetchSession {
let mut p = spawn_bash_with_retries(10, Some(self.session_timeout_ms))
.unwrap_or_else(|err| panic_any(err));

let cache_dir_path = format!("{:?}", self.cache_dir.path());

let paramfetch_path = cargo_bin("paramfetch");

let whitelist: String = self
.whitelisted_sector_sizes
.map(|wl| {
let mut s = "--sector-sizes=".to_string();
s.push_str(&wl.join(","));
s
})
.unwrap_or_else(|| "".to_string());

let json_argument = if self.manifest.is_some() {
format!("--json={:?}", self.manifest.expect("missing manifest"))
} else {
"".to_string()
let session = match spawn_bash_with_retries(10, Some(self.session_timeout_ms)) {
vmx marked this conversation as resolved.
Show resolved Hide resolved
Err(e) => panic_any(e),
Ok(mut session) => {
let cache_dir_path = format!("{:?}", self.cache_dir.path());

let paramfetch_path = cargo_bin("paramfetch");

let whitelist: String = self
.whitelisted_sector_sizes
.map(|wl| {
let mut s = "--sector-sizes=".to_string();
s.push_str(&wl.join(","));
s
})
.unwrap_or_else(|| "".to_string());

let json_argument = if self.manifest.is_some() {
format!("--json={:?}", self.manifest.expect("missing manifest"))
} else {
"".to_string()
};

let cmd = format!(
"{}={} {:?} {} {} {}",
"FIL_PROOFS_PARAMETER_CACHE", // related to var name in core/src/settings.rs
cache_dir_path,
paramfetch_path,
if self.prompt_enabled { "" } else { "--all" },
json_argument,
whitelist,
);

session
.execute(&cmd, ".*")
.expect("could not execute paramfetch");
session
}
};

let cmd = format!(
"{}={} {:?} {} {} {}",
"FIL_PROOFS_PARAMETER_CACHE", // related to var name in core/src/settings.rs
cache_dir_path,
paramfetch_path,
if self.prompt_enabled { "" } else { "--all" },
json_argument,
whitelist,
);

p.execute(&cmd, ".*").expect("could not execute paramfetch");

ParamFetchSession {
pty_session: p,
pty_session: session,
_cache_dir: self.cache_dir,
}
}
Expand Down
44 changes: 24 additions & 20 deletions fil-proofs-param/tests/parampublish/support/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,35 +116,39 @@ impl ParamPublishSessionBuilder {

/// Launch parampublish in an environment configured by the builder.
pub fn build(self) -> (ParamPublishSession, Vec<PathBuf>) {
let mut p = spawn_bash_with_retries(10, Some(self.session_timeout_ms))
.unwrap_or_else(|err| panic_any(err));

let cache_dir_path = format!("{:?}", self.cache_dir.path());
let session = match spawn_bash_with_retries(10, Some(self.session_timeout_ms)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The same comment as above applies here.

Copy link
Contributor Author

@storojs72 storojs72 May 31, 2022

Choose a reason for hiding this comment

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

Ok. Refactored in in 9af5c14

Err(err) => panic_any(err),
Ok(mut session) => {
let cache_dir_path = format!("{:?}", self.cache_dir.path());

let parampublish_path = cargo_bin("parampublish");

let cmd = format!(
"{}={} {:?} {} --ipfs-bin={:?} --json={:?}",
"FIL_PROOFS_PARAMETER_CACHE", // related to var name in core/src/settings.rs
cache_dir_path,
parampublish_path,
if self.list_all_files { "-a" } else { "" },
self.ipfs_bin_path,
self.manifest
);

session
.execute(&cmd, ".*")
.expect("could not execute parampublish");
session
}
};

let cache_contents: Vec<PathBuf> = read_dir(&self.cache_dir)
.unwrap_or_else(|_| panic_any(format!("failed to read cache dir {:?}", self.cache_dir)))
.map(|x| x.expect("failed to get dir entry"))
.map(|x| x.path())
.collect();

let parampublish_path = cargo_bin("parampublish");

let cmd = format!(
"{}={} {:?} {} --ipfs-bin={:?} --json={:?}",
"FIL_PROOFS_PARAMETER_CACHE", // related to var name in core/src/settings.rs
cache_dir_path,
parampublish_path,
if self.list_all_files { "-a" } else { "" },
self.ipfs_bin_path,
self.manifest
);

p.execute(&cmd, ".*")
.expect("could not execute parampublish");

(
ParamPublishSession {
pty_session: p,
pty_session: session,
_cache_dir: self.cache_dir,
},
cache_contents,
Expand Down
1 change: 0 additions & 1 deletion fil-proofs-param/tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ impl FakeIpfsBin {
pub fn compute_checksum<P: AsRef<Path>>(&self, path: P) -> Result<String, failure::Error> {
let output = Command::new(&self.bin_path)
.arg("add")
.arg("-Q")
.arg(path.as_ref())
.output()?;

Expand Down
2 changes: 1 addition & 1 deletion fil-proofs-tooling/src/bin/check_parameters/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::{Arg, Command};
use storage_proofs_core::parameter_cache::read_cached_params;

fn run_map(parameter_file: &Path) -> Result<MappedParameters<Bls12>> {
read_cached_params(&parameter_file.to_path_buf())
read_cached_params(parameter_file)
}

fn main() {
Expand Down
1 change: 1 addition & 0 deletions fil-proofs-tooling/src/bin/gpu-cpu-test/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl FromStr for Mode {
}

#[derive(Debug)]
#[allow(dead_code)]
pub struct RunInfo {
elapsed: Duration,
iterations: u8,
Expand Down
8 changes: 1 addition & 7 deletions filecoin-hashers/src/poseidon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl Hashable<PoseidonFunction> for PoseidonDomain {
}
}

#[derive(Copy, Clone, Debug, Serialize, Deserialize)]
#[derive(Default, Copy, Clone, Debug, Serialize, Deserialize)]
pub struct PoseidonDomain(pub <Fr as PrimeField>::Repr);

impl AsRef<PoseidonDomain> for PoseidonDomain {
Expand All @@ -79,12 +79,6 @@ impl PartialEq for PoseidonDomain {

impl Eq for PoseidonDomain {}

impl Default for PoseidonDomain {
fn default() -> PoseidonDomain {
PoseidonDomain(<Fr as PrimeField>::Repr::default())
}
}

impl Ord for PoseidonDomain {
#[inline(always)]
fn cmp(&self, other: &PoseidonDomain) -> Ordering {
Expand Down
4 changes: 1 addition & 3 deletions filecoin-proofs/tests/pieces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ fn test_get_piece_alignment() {
(300, 300, (208, 208)),
];

for (bytes_in_sector, bytes_in_piece, (expected_left_align, expected_right_align)) in
table.clone()
{
for (bytes_in_sector, bytes_in_piece, (expected_left_align, expected_right_align)) in table {
let PieceAlignment {
left_bytes: UnpaddedBytesAmount(actual_left_align),
right_bytes: UnpaddedBytesAmount(actual_right_align),
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.56.0
1.59.0
10 changes: 3 additions & 7 deletions sha2raw/src/sha256_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ use x86::{
pub unsafe fn compress256(state: &mut [u32; 8], blocks: &[&[u8]]) {
assert_eq!(blocks.len() % 2, 0);

let mut state0: __m128i;
let mut state1: __m128i;

let mut msg: __m128i;
let mut tmp: __m128i;

let mut msg0: __m128i;
let mut msg1: __m128i;
Expand All @@ -39,12 +35,12 @@ pub unsafe fn compress256(state: &mut [u32; 8], blocks: &[&[u8]]) {
);

// Load initial values
tmp = _mm_loadu_si128(state.as_ptr().add(0) as *const __m128i);
state1 = _mm_loadu_si128(state.as_ptr().add(4) as *const __m128i);
let mut tmp = _mm_loadu_si128(state.as_ptr().add(0) as *const __m128i);
vmx marked this conversation as resolved.
Show resolved Hide resolved
let mut state1 = _mm_loadu_si128(state.as_ptr().add(4) as *const __m128i);

tmp = _mm_shuffle_epi32(tmp, 0xB1); // CDAB
state1 = _mm_shuffle_epi32(state1, 0x1B); // EFGH
state0 = _mm_alignr_epi8(tmp, state1, 8); // ABEF
let mut state0 = _mm_alignr_epi8(tmp, state1, 8); // ABEF
state1 = _mm_blend_epi16(state1, tmp, 0xF0); // CDGH

for i in (0..blocks.len()).step_by(2) {
Expand Down
3 changes: 1 addition & 2 deletions storage-proofs-core/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ pub fn reverse_bit_numbering(bits: Vec<Boolean>) -> Vec<Boolean> {

padded_bits
.chunks(8)
.map(|chunk| chunk.iter().rev())
.flatten()
.flat_map(|chunk| chunk.iter().rev())
.cloned()
.collect()
}
Expand Down
1 change: 1 addition & 0 deletions storage-proofs-porep/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![deny(clippy::all, clippy::perf, clippy::correctness, rust_2018_idioms)]
#![allow(clippy::mut_from_ref)]
vmx marked this conversation as resolved.
Show resolved Hide resolved
#![warn(clippy::unwrap_used)]
#![cfg_attr(target_arch = "aarch64", feature(stdsimd))]
#![warn(clippy::unnecessary_wraps)]
Expand Down
2 changes: 1 addition & 1 deletion storage-proofs-porep/src/stacked/vanilla/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl ParentCache {
let mut digest_hex: String = "".to_string();
let sector_size = graph.size() * NODE_SIZE;

with_exclusive_lock(&path.to_path_buf(), |file| {
with_exclusive_lock(path, |file| {
let cache_size = cache_entries as usize * NODE_BYTES * DEGREE;
file.as_ref()
.set_len(cache_size as u64)
Expand Down
10 changes: 5 additions & 5 deletions storage-proofs-porep/src/stacked/vanilla/create_label/multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ fn fill_buffer(
cur_node: u64,
parents_cache: &CacheReader<u32>,
mut cur_parent: &[u32], // parents for this node
layer_labels: &UnsafeSlice<'_, u32>,
exp_labels: Option<&UnsafeSlice<'_, u32>>, // None for layer0
layer_labels: &UnsafeSlice<u32>,
exp_labels: Option<&UnsafeSlice<u32>>, // None for layer0
buf: &mut [u8],
base_parent_missing: &mut BitMask,
) {
Expand Down Expand Up @@ -140,15 +140,15 @@ fn fill_buffer(
#[allow(clippy::too_many_arguments)]
fn create_label_runner(
parents_cache: &CacheReader<u32>,
layer_labels: &UnsafeSlice<'_, u32>,
exp_labels: Option<&UnsafeSlice<'_, u32>>, // None for layer 0
layer_labels: &UnsafeSlice<u32>,
exp_labels: Option<&UnsafeSlice<u32>>, // None for layer 0
num_nodes: u64,
cur_producer: &AtomicU64,
cur_awaiting: &AtomicU64,
stride: u64,
lookahead: u64,
ring_buf: &RingBuf,
base_parent_missing: &UnsafeSlice<'_, BitMask>,
base_parent_missing: &UnsafeSlice<BitMask>,
) {
info!("created label runner");
// Label data bytes per node
Expand Down
4 changes: 2 additions & 2 deletions storage-proofs-porep/src/stacked/vanilla/proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,15 +578,15 @@ impl<'a, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> StackedDrg<'a, Tr
];

// gather all layer data.
for (layer_index, mut layer_bytes) in
for (layer_index, layer_bytes) in
layer_data.iter_mut().enumerate()
{
let store = labels.labels_for_layer(layer_index + 1);
let start = (i * nodes_count) + node_index;
let end = start + chunked_nodes_count;

store
.read_range_into(start, end, &mut layer_bytes)
.read_range_into(start, end, layer_bytes)
.expect("failed to read store range");
}

Expand Down
21 changes: 9 additions & 12 deletions storage-proofs-porep/src/stacked/vanilla/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,41 @@ use std::slice::{self, ChunksExactMut};
/// A slice type which can be shared between threads, but must be fully managed by the caller.
/// Any synchronization must be ensured by the caller, which is why all access is `unsafe`.
#[derive(Debug)]
pub struct UnsafeSlice<'a, T> {
// holds the data to ensure lifetime correctness
data: UnsafeCell<&'a mut [T]>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure about this one, but this doesn't seem right. As the comment suggests, I think we still want to have a reference to the data for correctness purpose. Hence I would just rename it to _data instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, keeping this lifetime is important, to make sure it is tracked properly

Copy link
Contributor Author

@storojs72 storojs72 May 31, 2022

Choose a reason for hiding this comment

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

Ok. Returned lifetime and renamed data to _data in 9af5c14. Could You please elaborate more on proper tracking?

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that if we just use the raw pointer the compiler won't be able to track the lifetime for us. if we keep it in here we get the tracking for it.

pub struct UnsafeSlice<T> {
/// pointer to the data
ptr: *mut T,
/// Number of elements, not bytes.
len: usize,
}

unsafe impl<'a, T> Sync for UnsafeSlice<'a, T> {}
unsafe impl<T> Sync for UnsafeSlice<T> {}

impl<'a, T> UnsafeSlice<'a, T> {
impl<T> UnsafeSlice<T> {
/// Takes mutable slice, to ensure that `UnsafeSlice` is the only user of this memory, until it gets dropped.
pub fn from_slice(source: &'a mut [T]) -> Self {
pub fn from_slice(source: &mut [T]) -> Self {
let len = source.len();
let ptr = source.as_mut_ptr();
let data = UnsafeCell::new(source);
Self { data, ptr, len }
Self { ptr, len }
}

/// Safety: The caller must ensure that there are no unsynchronized parallel access to the same regions.
#[inline]
pub unsafe fn as_mut_slice(&self) -> &'a mut [T] {
pub unsafe fn as_mut_slice(&self) -> &mut [T] {
slice::from_raw_parts_mut(self.ptr, self.len)
}
/// Safety: The caller must ensure that there are no unsynchronized parallel access to the same regions.
#[inline]
pub unsafe fn as_slice(&self) -> &'a [T] {
pub unsafe fn as_slice(&self) -> &[T] {
slice::from_raw_parts(self.ptr, self.len)
}

#[inline]
pub unsafe fn get(&self, index: usize) -> &'a T {
pub unsafe fn get(&self, index: usize) -> &T {
&*self.ptr.add(index)
}

#[inline]
pub unsafe fn get_mut(&self, index: usize) -> &'a mut T {
pub unsafe fn get_mut(&self, index: usize) -> &mut T {
&mut *self.ptr.add(index)
}
}
Expand Down