perf: Reduce TASO hashtable size #133

lmondada · 2023-09-25T15:05:31Z

The idea of this PR is to store hashes of seen circuits in buckets given by the gate count of the circuit. That way hashes above a certain gate count can be cleared.

@aborgna-q I had to remove the call to tracing::trace_span. Where should I re-introduce it in the new code?

EDIT: I had run tests to compare memory usage, but the variation between runs was too big for it to mean anything. Not sure how to track memory usage well enough.

lmondada · 2023-09-26T13:55:49Z

After some more testing (and one very crucial improvement, see latest commit), improvements are very clear. These are run with PRIORITY_QUEUE_CAPACITY = 500 to make the difference more obvious on short-ish time scales.

Command: cargo run --release -- -j1 --eccs ../test_files/Nam_6_3_complete_ECC_set.json -i ../bench-vs-quartz/circuits/barenco_tof_5.json -t120

On main

Tried 31941 circuits
END RESULT: 183
Saving result
Peak memory usage: 1.9340135 GB
Done.

This PR:

Tried 18078 circuits
END RESULT: 178
Saving result
Peak memory usage: 0.98692214 GB
Done.

aborgna-q

Nice!

The changes to the priority channel will also be useful as a base for the sharding idea.

aborgna-q · 2023-09-27T08:41:01Z

src/optimiser/taso.rs

+                if (pq.len() > PRIORITY_QUEUE_CAPACITY / 2
+                    && new_circ_cost > *pq.max_cost().unwrap())


Check this before computing the hash, so we may skip that computation on some cases.

aborgna-q · 2023-09-27T08:42:08Z

src/optimiser/taso.rs

@@ -130,18 +131,23 @@ where
            let rewrites = self.rewriter.get_rewrites(&circ);
            for new_circ in self.strategy.apply_rewrites(rewrites, &circ) {
                let new_circ_hash = new_circ.circuit_hash();
+                let new_circ_cost = (self.cost)(&new_circ);
                circ_cnt += 1;


Looking at this; do we want to count repeated hashes as seen multiple times? Otherwise this should go after the branch.

src/optimiser/taso/hugr_pchannel.rs

aborgna-q · 2023-09-27T09:26:11Z

src/optimiser/taso/hugr_pchannel.rs

+                                self.log
+                                    .send(PriorityChannelLog::CircuitCount(
+                                        self.circ_cnt,
+                                        self.seen_hashes.len(),
+                                    ))
+                                    .unwrap();


Move this out of the loop, so other breaks also trigger a last log.

aborgna-q · 2023-09-27T09:37:06Z

src/optimiser/taso/hugr_pchannel.rs

+            self.circ_cnt += 1;
+            if self.circ_cnt % 1000 == 0 {
+                // TODO: Add a minimum time between logs
+                self.log


We could log directly from this thread, but currently TasoLogger is non-copyable so we cannot share it.

LGTM for now, but we'll probably want to simplify it later.

Yes I agree.

aborgna-q · 2023-09-27T09:46:19Z

src/optimiser/taso/hugr_hash_set.rs

+            self.buckets.push_front([hash].into_iter().collect());
+            return true;
+        };
+        while cost < *min_cost {


Suggested change

while cost < *min_cost {

self.buckets.reserve(min_cost.saturating_sub(cost));

while cost < *min_cost {

aborgna-q · 2023-09-27T09:52:03Z

src/optimiser/taso/hugr_hash_set.rs

+            *min_cost -= 1;
+        }
+        let bucket_index = cost - *min_cost;
+        while bucket_index >= self.buckets.len() {


Suggested change

while bucket_index >= self.buckets.len() {

let missing_back = (bucket_index+1).saturating_sub(self.buckets.len());

self.buckets.reserve(missing_back);

while bucket_index >= self.buckets.len() {

Or alternatively

let missing_back = (bucket_index+1).saturating_sub(self.buckets.len()); self.buckets.extend(iter::repeat_with(|| FxHashSet::default()).take(missing_back));

or even

if bucket_index >= self.buckets.len() { self.buckets.resize_with(bucket_index + 1, FxHashSet::default); }

aborgna-q · 2023-09-27T09:53:39Z

src/optimiser/taso/hugr_hash_set.rs

+        self.buckets[bucket_index].insert(hash)
+    }
+
+    // /// Returns whether the given hash is present in the set.


Suggested change

// /// Returns whether the given hash is present in the set.

/// Returns whether the given hash is present in the set.

Co-authored-by: Agustín Borgna <[email protected]>

This reverts commit e33fc6a.

lmondada added 3 commits September 25, 2023 17:01

perf: Reduce TASO hashtable size

60bf9e3

contains -> insert

d1862d8

Do not pollute hash table with expensive cost

5f0c35c

lmondada mentioned this pull request Sep 26, 2023

feat!: Use CX count as default cost function #134

Merged

aborgna-q approved these changes Sep 27, 2023

View reviewed changes

lmondada and others added 3 commits September 27, 2023 13:53

Update src/optimiser/taso/hugr_pchannel.rs

a6255ef

Co-authored-by: Agustín Borgna <[email protected]>

Address comments

a3d31b0

Merge branch 'main' into fix/reduce-hash-memory

7e3050f

lmondada merged commit e33fc6a into main Sep 27, 2023

lmondada deleted the fix/reduce-hash-memory branch September 27, 2023 12:16

lmondada added a commit that referenced this pull request Sep 28, 2023

Revert "perf: Reduce TASO hashtable size (#133)"

7c2d511

This reverts commit e33fc6a.

This was referenced Sep 28, 2023

Reduce TASO hashtable size #145

Closed

chore: Move hashing to queueing thread #156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: Reduce TASO hashtable size #133

perf: Reduce TASO hashtable size #133

lmondada commented Sep 25, 2023 •

edited

Loading

lmondada commented Sep 26, 2023 •

edited

Loading

aborgna-q left a comment

aborgna-q Sep 27, 2023

aborgna-q Sep 27, 2023

aborgna-q Sep 27, 2023

aborgna-q Sep 27, 2023

lmondada Sep 27, 2023

aborgna-q Sep 27, 2023

aborgna-q Sep 27, 2023

lmondada Sep 27, 2023

aborgna-q Sep 27, 2023

		if (pq.len() > PRIORITY_QUEUE_CAPACITY / 2
		&& new_circ_cost > *pq.max_cost().unwrap())

	while cost < *min_cost {
	self.buckets.reserve(min_cost.saturating_sub(cost));
	while cost < *min_cost {

	// /// Returns whether the given hash is present in the set.
	/// Returns whether the given hash is present in the set.

perf: Reduce TASO hashtable size #133

perf: Reduce TASO hashtable size #133

Conversation

lmondada commented Sep 25, 2023 • edited Loading

lmondada commented Sep 26, 2023 • edited Loading

aborgna-q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmondada commented Sep 25, 2023 •

edited

Loading

lmondada commented Sep 26, 2023 •

edited

Loading