initialize recycled data #967
Conversation
| } else { | ||
| x | ||
|
|
||
| loop { |
There was a problem hiding this comment.
I had to drop recursion here when I was using default::Default in my initial SWAG at this; I learned that Rust apparently isn't smart about tail recursion, and in our tests (where a lot of strong-ref'd blobs are in the recycler) 64k per stack frame was being allocated, we recursed WINDOW_SIZE frames, and it completely blew up (no stack backtrace or nothin').
I prefer the loop form, though. I'm not a Haskell guy ;)
There was a problem hiding this comment.
It's probably LLVM that's the problem, not Rust. Haskell's GHC had to go add another calling convention to LLVM to allow for tail recursion. The iterative version is fine my me. If you want to write a test for that, looks like thread::Builder lets you control the thread's stack size:
let builder = thread::Builder::new()
.name("kaboom".into())
.stack_size(32 * 1024 * 1024); // 32MB of stack space
let handler = builder.spawn(|| {
// stack-intensive operations
}).unwrap();
handler.join() // <--- assert this returns an Err
There was a problem hiding this comment.
👍 on iterative version :-)
src/packet.rs
Outdated
| @@ -63,6 +63,19 @@ impl Default for Packet { | |||
| } | |||
| } | |||
|
|
|||
| pub trait Recycle { | |||
There was a problem hiding this comment.
I would have called this Recyclable, but I don't see a precedent for that "-able" naming convention in Rust, which is too bad. Looks like Rustaceans would name it Reset. Thoughts?
There was a problem hiding this comment.
Reset works for me, too. I didn't think -able looked right.
src/packet.rs
Outdated
| // Recycle trait is an object that can re-initialize important parts | ||
| // of itself, similar to Default, but not necessarily a full clear | ||
| // also, we do it in-place. | ||
| fn reset(&mut self) -> (); |
There was a problem hiding this comment.
Did that -> () get by Clippy? Can you remove it?
There was a problem hiding this comment.
clippy keeps going away on my box(es) ...rust updates?
There was a problem hiding this comment.
looks like clippy doesn't care, though https://github.com/solana-labs/solana/blob/master/src/crdt.rs#L338
| @@ -550,7 +550,7 @@ mod tests { | |||
|
|
|||
| fn tmp_ledger_path(name: &str) -> String { | |||
| use std::env; | |||
| let out_dir = env::var("OUT_DIR").unwrap(); | |||
| let out_dir = env::var("OUT_DIR").unwrap_or_else(|_| "target".to_string()); | |||
There was a problem hiding this comment.
should be a function get_out_dir() -> String which can share this logic?
There was a problem hiding this comment.
yeah, eventually I'd like to collapse temp_dir stuff to a single implementation. I had to hurry up and throw this in because for some reason cargo stopped offering "OUT_DIR" in some of my test runs (like cargo watch -x test, maybe)...
| impl Reset for Blob { | ||
| fn reset(&mut self) { | ||
| self.meta = Meta::default(); | ||
| self.data[..BLOB_HEADER_SIZE].copy_from_slice(&[0u8; BLOB_HEADER_SIZE]); |
There was a problem hiding this comment.
TODO: probably non-optimal memset
There was a problem hiding this comment.
annoyingly doesn't seem like rust has a safe one at the moment
There was a problem hiding this comment.
what's a better memset() look like?
There was a problem hiding this comment.
unsafe {
ptr::write_bytes(self.slice.as_mut_ptr(), 0, self.slice.len() - 1);
}
I assume this is doing an allocation with a slice of 0 bytes and then doing a memcpy, but allocation is not needed just zero fill with simd movs.
| } else { | ||
| x | ||
|
|
||
| loop { |
There was a problem hiding this comment.
👍 on iterative version :-)
…olana-labs#967) * Automate primordial accounts * Add genesis account dumping as test * Add test-dump-genesis-accounts feature flag to create state * Run cargo fmt * Delete gitignore * Add test-bpf feature to genesis test Co-authored-by: Justin Starry <justin@solana.com>
…_window (backport of #904) (#967) * quic: delay calling set_max_concurrent_uni_streams/set_receive_window (#904) * quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection Avoids taking a mutex and waking a task. * quic: don't increase the receive window before we've actually accepted a connection (cherry picked from commit 2770424) # Conflicts: # streamer/src/nonblocking/quic.rs * Fix merge conflicts --------- Co-authored-by: Alessandro Decina <alessandro.d@gmail.com>
…_window (backport of solana-labs#904) (solana-labs#967) * quic: delay calling set_max_concurrent_uni_streams/set_receive_window (solana-labs#904) * quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection Avoids taking a mutex and waking a task. * quic: don't increase the receive window before we've actually accepted a connection (cherry picked from commit 2770424) # Conflicts: # streamer/src/nonblocking/quic.rs * Fix merge conflicts --------- Co-authored-by: Alessandro Decina <alessandro.d@gmail.com>
No description provided.