Skip to content

Commit 319d643

Browse files
swernlicesarzc
andauthored
Optimize evaluator perf via Control Flow Graph (#1261)
This change adds new content to FIR that captures the control flow graph for expressions. With this pre-calculated during lowering, the evaluator is refactored to follow that sequencing rather than calculating (and recalculating) the execution sequence on the fly. In addition to the performance numbers from the CI benchmarks (which show larger gains in the Linux container), here is some data gathered from runs on my machine. This change tested in WASM shows overall performance benefit but <s>with a trade off of reduced performance on large array literals</s> with the latest change also improving performance on array literals: <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta name=ProgId content=Excel.Sheet> <meta name=Generator content="Microsoft Excel 15"> <link id=Main-File rel=Main-File href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip.htm"> <link rel=File-List href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip_filelist.xml"> </head> <body link="#467886" vlink="#96607D"> WASI (time in µs) |   |   |   -- | -- | -- | -- Benchmark | Main | PR | Perf Impact Teleport eval | 162.55 | 129.86 | -20.11% DJ eval | 9835.53 | 7910.8 | -19.57% Large File eval | 37323 | 35022 | -6.17% Array Append | 1835 | 1265.6 | -31.03% Array Update | 2089.2 | 1556 | -25.52% Array Literal | 694.17 | 377.02 | -45.69% Large Nested | 195790 | 135720 | -30.68% </body> </html> The performance gains in native code are more significant for those benchmarks that do large numbers of iterations through code. Both the "Main" and "PR" below include the mimalloc optimization that has already merged: <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta name=ProgId content=Excel.Sheet> <meta name=Generator content="Microsoft Excel 15"> <link id=Main-File rel=Main-File href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip.htm"> <link rel=File-List href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip_filelist.xml"> </head> <body link="#467886" vlink="#96607D"> Native (time in µs) |   |   |   -- | -- | -- | -- Benchmark | Main | PR | Perf Impact Teleport eval | 39.034 | 32.269 | -17.33% DJ eval | 2650.8 | 2220.6 | -16.23% Large File eval | 25327 | 23411 | -7.57% Array Append | 255.74 | 152.52 | -40.36% Array Update | 254.13 | 170.67 | -32.84% Array Literal | 117.93 | 79.975 | -32.18% Large Nested | 25544 | 14966 | -41.41% </body> </html> Finally, the performance gains for these combined changes on resource estimation workloads show promise in the larger cases, specifically: <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta name=ProgId content=Excel.Sheet> <meta name=Generator content="Microsoft Excel 15"> <link id=Main-File rel=Main-File href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip.htm"> <link rel=File-List href="file:////Users/swernli/Library/Group%20Containers/UBF8T346G9.Office/TemporaryItems/msohtmlclip/clip_filelist.xml"> </head> <body link="#467886" vlink="#96607D"> Resource Estimation (time in sec) |   |   |   -- | -- | -- | -- Workload | v1.2 | PR | Perf Impact dfchem XVIII-cas4-fb-64e-56o | 45 | 30 | -33.33% dfchem nitrogenase-54e-54o | 123 | 80 | -34.96% Windowed multiplication* | 548 | 333 | -39.23% </body> </html> *from https://arxiv.org/abs/2402.01891 --------- Co-authored-by: César Zaragoza Cortés <[email protected]>
1 parent 98c73bf commit 319d643

File tree

16 files changed

+1435
-790
lines changed

16 files changed

+1435
-790
lines changed

allocator/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ edition.workspace = true
77
license.workspace = true
88
version.workspace = true
99

10-
[dependencies]
10+
[target.'cfg(not(target_family = "wasm"))'.dependencies]
1111
mimalloc-sys = { path = "./mimalloc-sys" }
1212

1313
[lints]

compiler/qsc/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ qsc_passes = { path = "../qsc_passes" }
3030
qsc_project = { path = "../qsc_project", features = ["fs"] }
3131
rustc-hash = { workspace = true }
3232
thiserror = { workspace = true }
33-
34-
[target.'cfg(not(any(target_family = "wasm")))'.dependencies]
3533
allocator = { path = "../../allocator" }
3634

3735
[dev-dependencies]

compiler/qsc/src/interpret.rs

Lines changed: 55 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,13 @@ mod tests;
99
#[cfg(test)]
1010
mod debugger_tests;
1111

12+
use std::rc::Rc;
13+
1214
pub use qsc_eval::{
1315
debug::Frame,
1416
output::{self, GenericReceiver},
17+
val::Closure,
18+
val::Range as ValueRange,
1519
val::Result,
1620
val::Value,
1721
StepAction, StepResult,
@@ -37,9 +41,9 @@ use qsc_eval::{
3741
debug::{map_fir_package_to_hir, map_hir_package_to_fir},
3842
output::Receiver,
3943
val::{self},
40-
Env, EvalId, State, VariableInfo,
44+
Env, State, VariableInfo,
4145
};
42-
use qsc_fir::fir::{self, Global, PackageStoreLookup};
46+
use qsc_fir::fir::{self, ExecGraphNode, Global, PackageStoreLookup};
4347
use qsc_fir::{
4448
fir::{Block, BlockId, Expr, ExprId, Package, PackageId, Pat, PatId, Stmt, StmtId},
4549
visit::{self, Visitor},
@@ -173,11 +177,11 @@ impl Interpreter {
173177
&mut self,
174178
receiver: &mut impl Receiver,
175179
) -> std::result::Result<Value, Vec<Error>> {
176-
let expr = self.get_entry_expr()?;
180+
let graph = self.get_entry_exec_graph()?;
177181
eval(
178182
self.source_package,
179183
self.classical_seed,
180-
expr.into(),
184+
graph,
181185
self.compiler.package_store(),
182186
&self.fir_store,
183187
&mut Env::default(),
@@ -193,14 +197,14 @@ impl Interpreter {
193197
sim: &mut impl Backend<ResultType = impl Into<val::Result>>,
194198
receiver: &mut impl Receiver,
195199
) -> std::result::Result<Value, Vec<Error>> {
196-
let expr = self.get_entry_expr()?;
200+
let graph = self.get_entry_exec_graph()?;
197201
if self.quantum_seed.is_some() {
198202
sim.set_seed(self.quantum_seed);
199203
}
200204
eval(
201205
self.source_package,
202206
self.classical_seed,
203-
expr.into(),
207+
graph,
204208
self.compiler.package_store(),
205209
&self.fir_store,
206210
&mut Env::default(),
@@ -209,10 +213,10 @@ impl Interpreter {
209213
)
210214
}
211215

212-
fn get_entry_expr(&self) -> std::result::Result<ExprId, Vec<Error>> {
216+
fn get_entry_exec_graph(&self) -> std::result::Result<Rc<[ExecGraphNode]>, Vec<Error>> {
213217
let unit = self.fir_store.get(self.source_package);
214-
if let Some(entry) = unit.entry {
215-
return Ok(entry);
218+
if unit.entry.is_some() {
219+
return Ok(unit.entry_exec_graph.clone());
216220
};
217221
Err(vec![Error::NoEntryPoint])
218222
}
@@ -233,7 +237,7 @@ impl Interpreter {
233237
.compile_fragments_fail_fast(&label, fragments)
234238
.map_err(into_errors)?;
235239

236-
let stmts = self.lower(&increment);
240+
let (_, graph) = self.lower(&increment);
237241

238242
// Updating the compiler state with the new AST/HIR nodes
239243
// is not necessary for the interpreter to function, as all
@@ -243,22 +247,16 @@ impl Interpreter {
243247
// here to keep the package stores consistent.
244248
self.compiler.update(increment);
245249

246-
let mut result = Value::unit();
247-
248-
for stmt_id in stmts {
249-
result = eval(
250-
self.package,
251-
self.classical_seed,
252-
stmt_id.into(),
253-
self.compiler.package_store(),
254-
&self.fir_store,
255-
&mut self.env,
256-
&mut self.sim,
257-
receiver,
258-
)?;
259-
}
260-
261-
Ok(result)
250+
eval(
251+
self.package,
252+
self.classical_seed,
253+
graph.into(),
254+
self.compiler.package_store(),
255+
&self.fir_store,
256+
&mut self.env,
257+
&mut self.sim,
258+
receiver,
259+
)
262260
}
263261

264262
/// Runs the given entry expression on a new instance of the environment and simulator,
@@ -300,7 +298,7 @@ impl Interpreter {
300298
receiver: &mut impl Receiver,
301299
expr: &str,
302300
) -> std::result::Result<InterpretResult, Vec<Error>> {
303-
let expr_id = self.compile_entry_expr(expr)?;
301+
let graph = self.compile_entry_expr(expr)?;
304302

305303
if self.quantum_seed.is_some() {
306304
sim.set_seed(self.quantum_seed);
@@ -309,7 +307,7 @@ impl Interpreter {
309307
Ok(eval(
310308
self.package,
311309
self.classical_seed,
312-
expr_id.into(),
310+
graph.into(),
313311
self.compiler.package_store(),
314312
&self.fir_store,
315313
&mut Env::default(),
@@ -318,15 +316,18 @@ impl Interpreter {
318316
))
319317
}
320318

321-
fn compile_entry_expr(&mut self, expr: &str) -> std::result::Result<ExprId, Vec<Error>> {
319+
fn compile_entry_expr(
320+
&mut self,
321+
expr: &str,
322+
) -> std::result::Result<Vec<ExecGraphNode>, Vec<Error>> {
322323
let increment = self
323324
.compiler
324325
.compile_entry_expr(expr)
325326
.map_err(into_errors)?;
326327

327328
// `lower` will update the entry expression in the FIR store,
328329
// and it will always return an empty list of statements.
329-
let _ = self.lower(&increment);
330+
let (_, graph) = self.lower(&increment);
330331

331332
// The AST and HIR packages in `increment` only contain an entry
332333
// expression and no statements. The HIR *can* contain items if the entry
@@ -342,16 +343,19 @@ impl Interpreter {
342343
// here to keep the package stores consistent.
343344
self.compiler.update(increment);
344345

345-
let unit = self.fir_store.get(self.package);
346-
let entry = unit.entry.expect("package should have an entry expression");
347-
348-
Ok(entry)
346+
Ok(graph)
349347
}
350348

351-
fn lower(&mut self, unit_addition: &qsc_frontend::incremental::Increment) -> Vec<StmtId> {
349+
fn lower(
350+
&mut self,
351+
unit_addition: &qsc_frontend::incremental::Increment,
352+
) -> (Vec<StmtId>, Vec<ExecGraphNode>) {
352353
let fir_package = self.fir_store.get_mut(self.package);
353-
self.lowerer
354-
.lower_and_update_package(fir_package, &unit_addition.hir)
354+
(
355+
self.lowerer
356+
.lower_and_update_package(fir_package, &unit_addition.hir),
357+
self.lowerer.take_exec_graph(),
358+
)
355359
}
356360

357361
fn next_line_label(&mut self) -> String {
@@ -387,24 +391,15 @@ impl Debugger {
387391
language_features,
388392
)?;
389393
let source_package_id = interpreter.source_package;
394+
let unit = interpreter.fir_store.get(source_package_id);
395+
let entry_exec_graph = unit.entry_exec_graph.clone();
390396
Ok(Self {
391397
interpreter,
392398
position_encoding,
393-
state: State::new(source_package_id, None),
399+
state: State::new(source_package_id, entry_exec_graph, None),
394400
})
395401
}
396402

397-
/// Loads the entry expression to the top of the evaluation stack.
398-
/// This is needed for debugging so that when begging to debug with
399-
/// a step action the system is already in the correct state.
400-
/// # Errors
401-
/// Returns a vector of errors if loading the entry point fails.
402-
pub fn set_entry(&mut self) -> std::result::Result<(), Vec<Error>> {
403-
let expr = self.interpreter.get_entry_expr()?;
404-
qsc_eval::eval_push_expr(&mut self.state, expr);
405-
Ok(())
406-
}
407-
408403
/// Resumes execution with specified `StepAction`.
409404
/// # Errors
410405
/// Returns a vector of errors if evaluating the entry point fails.
@@ -527,15 +522,23 @@ impl Debugger {
527522
fn eval(
528523
package: PackageId,
529524
classical_seed: Option<u64>,
530-
id: EvalId,
525+
exec_graph: Rc<[ExecGraphNode]>,
531526
package_store: &PackageStore,
532527
fir_store: &fir::PackageStore,
533528
env: &mut Env,
534529
sim: &mut impl Backend<ResultType = impl Into<val::Result>>,
535530
receiver: &mut impl Receiver,
536531
) -> InterpretResult {
537-
qsc_eval::eval(package, classical_seed, id, fir_store, env, sim, receiver)
538-
.map_err(|(error, call_stack)| eval_error(package_store, fir_store, call_stack, error))
532+
qsc_eval::eval(
533+
package,
534+
classical_seed,
535+
exec_graph,
536+
fir_store,
537+
env,
538+
sim,
539+
receiver,
540+
)
541+
.map_err(|(error, call_stack)| eval_error(package_store, fir_store, call_stack, error))
539542
}
540543

541544
/// Represents a stack frame for debugging.

compiler/qsc/src/interpret/debugger_tests.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ mod given_debugger {
135135
Encoding::Utf8,
136136
LanguageFeatures::default(),
137137
)?;
138-
debugger.set_entry()?;
139138
let ids = get_breakpoint_ids(&debugger, "test");
140139
let expected_id = ids[0];
141140
expect_bp(&mut debugger, &ids, expected_id);
@@ -159,7 +158,6 @@ mod given_debugger {
159158
Encoding::Utf8,
160159
LanguageFeatures::default(),
161160
)?;
162-
debugger.set_entry()?;
163161
let ids = get_breakpoint_ids(&debugger, "test");
164162
let expected_id = ids[0];
165163
expect_bp(&mut debugger, &ids, expected_id);
@@ -179,7 +177,6 @@ mod given_debugger {
179177
Encoding::Utf8,
180178
LanguageFeatures::default(),
181179
)?;
182-
debugger.set_entry()?;
183180
let ids = get_breakpoint_ids(&debugger, "test");
184181
let expected_id = ids[0];
185182
expect_bp(&mut debugger, &ids, expected_id);
@@ -206,7 +203,6 @@ mod given_debugger {
206203
Encoding::Utf8,
207204
LanguageFeatures::default(),
208205
)?;
209-
debugger.set_entry()?;
210206
let ids = get_breakpoint_ids(&debugger, "test");
211207
let expected_id = ids[0];
212208
expect_bp(&mut debugger, &ids, expected_id);

compiler/qsc_codegen/src/qir_base.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,14 @@ pub fn generate_qir(
4242

4343
let package = map_hir_package_to_fir(package);
4444
let unit = fir_store.get(package);
45-
let entry_expr = unit.entry.expect("package should have entry");
4645

4746
let mut sim = BaseProfSim::default();
4847
let mut stdout = std::io::sink();
4948
let mut out = GenericReceiver::new(&mut stdout);
5049
let result = eval(
5150
package,
5251
None,
53-
entry_expr.into(),
52+
unit.entry_exec_graph.clone(),
5453
&fir_store,
5554
&mut Env::default(),
5655
&mut sim,

compiler/qsc_eval/src/intrinsic/tests.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ use std::f64::consts;
77

88
use crate::backend::{Backend, SparseSim};
99
use crate::debug::map_hir_package_to_fir;
10-
use crate::tests::eval_expr;
10+
use crate::tests::eval_graph;
11+
use crate::Env;
1112
use crate::{
1213
output::{GenericReceiver, Receiver},
1314
val::Value,
@@ -180,7 +181,7 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
180181
)
181182
.is_empty());
182183
let unit_fir = fir_lowerer.lower_package(&unit.package);
183-
let entry = unit_fir.entry.expect("package should have entry");
184+
let entry = unit_fir.entry_exec_graph.clone();
184185

185186
let id = store.insert(unit);
186187

@@ -192,11 +193,12 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
192193
fir_store.insert(map_hir_package_to_fir(std_id), std_fir);
193194
fir_store.insert(map_hir_package_to_fir(id), unit_fir);
194195

195-
eval_expr(
196+
eval_graph(
196197
entry,
197198
&mut CustomSim::default(),
198199
&fir_store,
199200
map_hir_package_to_fir(id),
201+
&mut Env::default(),
200202
out,
201203
)
202204
.map_err(|e| e.0)

0 commit comments

Comments
 (0)