-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: Multithreading improvements #182
Conversation
src/optimiser/taso/hugr_pchannel.rs
Outdated
self.circ_cnt += 1; | ||
self.log | ||
.send(PriorityChannelLog::CircuitCount { | ||
processed_count: self.circ_cnt, | ||
seen_count: self.seen_hashes.len(), | ||
queue_length: self.pq.len(), | ||
}) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on purpose that you are sending a log at every circ_cnt
increment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally doesn't happen that frequently. I guess I could add a timeout.
pub struct Entry<C, P, H> { | ||
pub circ: C, | ||
pub cost: P, | ||
pub hash: H, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could/should this type be the same as type Work
in hugr_pchannel
?
src/optimiser/taso/log.rs
Outdated
workqueue_len: Option<usize>, | ||
seen_hashes: usize, | ||
) { | ||
if circ_cnt % 1000 == 0 { | ||
self.progress(format!("{circ_cnt} circuits...")); | ||
if circuits_processed > self.last_circ_processed && circuits_processed % 1000 == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be if circuits_processed >= self.last_circ_processed + 1000
src/optimiser/taso/worker.rs
Outdated
/// A unit of work for a worker, consisting of a circuit to process, along its | ||
/// hash and cost. | ||
pub type Work<P> = (P, u64, Hugr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've defined this twice.
The main feature of this PR is reducing the memory usage of multithreading with a priority channel by adding a shared
Arc<RwLock<Option<P>>>
with the maximum circuit cost in the queue, so the workers don't fill the channels with useless data.The main improvement of this is limiting the memory usage when multiple threads are involved. Now I can run
barenco_tof_10
on 10 threads with ~4GB of RAM, where before it exceeded 10GB (and caused a OOM).I also included multiple small changes extracted from #175;
CircuitCost::as_usize
instead of proxy operations, so we can share cost limits as atomics and run other primitive operations directly.eprintln!
s with log calls.