Skip to content

Commit

Permalink
chore: some minor improvements (#1210)
Browse files Browse the repository at this point in the history
* avoid some copies where we already require ownership
* use `car_cdr_simple` where we're not interested in deconstructing strings
* use `fetch_cons` where we explicitly require the pointer's tag to be `Cons`
  • Loading branch information
arthurpaulino authored Mar 11, 2024
1 parent 770b55d commit 7a2f3b2
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 138 deletions.
4 changes: 2 additions & 2 deletions benches/common/fib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ fn lurk_fib<F: LurkField>(store: &Store<F>, n: usize) -> Ptr {
// saved_env: (((.lurk.user.next . <FUNCTION (.lurk.user.a .lurk.user.b) (.lurk.user.next .lurk.user.b (+ .lurk.user.a .lurk.user.b))>))),
// body: (.lurk.user.fib), continuation: Outermost }

let [_, _, rest_bindings] = store.pop_binding(*target_env).unwrap();
let [_, val, _] = store.pop_binding(rest_bindings).unwrap();
let [_, _, rest_bindings] = store.pop_binding(target_env).unwrap();
let [_, val, _] = store.pop_binding(&rest_bindings).unwrap();
val
}

Expand Down
7 changes: 4 additions & 3 deletions chain-server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ where
// make sure that the evaluation terminated appropriatelly
match cont_out.tag() {
Tag::Cont(ContTag::Terminal) => {
// car_cdr the result to retrieve the chain result and the next callable
let (result, next_callable) = self.store.car_cdr(expr_out).map_err(|_e| {
Status::failed_precondition("Call didn't result in a cons-like expression")
// get the car/cdr of the result to retrieve the chain result and
// the next callable
let (result, next_callable) = self.store.fetch_cons(expr_out).ok_or_else(|| {
Status::failed_precondition("Call didn't result in a cons expression")
})?;

// retrieve (or compute if needed) the public params for proving
Expand Down
4 changes: 2 additions & 2 deletions foil/src/coil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub struct Context {
/// Look up `var` in `env`, where `env` is a list of bindings from `Symbol` to `U64` representing bindings of variables to vertices
/// by id.
fn lookup_vertex_id<F: LurkField>(store: &Store<F>, env: &Ptr, var: &Ptr) -> Result<Option<Id>> {
let Some([bound_var, id, rest_env]) = store.pop_binding(*env) else {
let Some([bound_var, id, rest_env]) = store.pop_binding(env) else {
return Ok(None);
};

Expand Down Expand Up @@ -125,7 +125,7 @@ impl Context {

fn pop_binding<F: LurkField>(&mut self, store: &Store<F>) -> Result<Id> {
let [_var, id, rest_env] = store
.pop_binding(self.env)
.pop_binding(&self.env)
.ok_or(anyhow!("failed to pop binding"))?;
self.env = rest_env;

Expand Down
29 changes: 16 additions & 13 deletions src/cli/repl/meta_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ where
example: &["!(defpackage abc)"],
run: |repl, args, _path| {
// TODO: handle args
let (name, _args) = repl.store.car_cdr(args)?;
let (name, _args) = repl.store.car_cdr_simple(args)?;
let name = match name.tag() {
Tag::Expr(ExprTag::Str) => repl.state.borrow_mut().intern(repl.get_string(&name)?),
Tag::Expr(ExprTag::Sym) => repl.get_symbol(&name)?.into(),
Expand All @@ -419,15 +419,15 @@ where
example: &[],
run: |repl, args, _path| {
// TODO: handle pkg
let (mut symbols, _pkg) = repl.store.car_cdr(args)?;
let (mut symbols, _pkg) = repl.store.car_cdr_simple(args)?;
if symbols.tag() == &Tag::Expr(ExprTag::Sym) {
let sym = SymbolRef::new(repl.get_symbol(&symbols)?);
repl.state.borrow_mut().import(&[sym])?;
} else {
let mut symbols_vec = vec![];
loop {
{
let (head, tail) = repl.store.car_cdr(&symbols)?;
let (head, tail) = repl.store.car_cdr_simple(&symbols)?;
let sym = repl.get_symbol(&head)?;
symbols_vec.push(SymbolRef::new(sym));
if tail.is_nil() {
Expand Down Expand Up @@ -530,7 +530,7 @@ where
}

fn call(repl: &mut Repl<F, C>, args: &Ptr, _path: &Utf8Path) -> Result<()> {
let (hash_expr, args) = repl.store.car_cdr(args)?;
let (hash_expr, args) = repl.store.car_cdr_simple(args)?;
let hash = *repl.get_comm_hash(hash_expr)?;
// check if the data is already available on the store before trying to
// fetch it from the file system
Expand Down Expand Up @@ -586,7 +586,10 @@ where
let result = ev
.get_result()
.expect("evaluation result must have been set");
let (_, comm) = repl.store.car_cdr(result)?;
let (_, comm) = repl
.store
.fetch_cons(result)
.ok_or_else(|| anyhow!("Chained function must return a cons expression"))?;
let (Tag::Expr(ExprTag::Comm), RawPtr::Atom(hash)) = comm.parts() else {
bail!("Second component of a chain must be a commitment")
};
Expand Down Expand Up @@ -719,9 +722,9 @@ where
" :description \"example protocol\")",
],
run: |repl, args, _path| {
let (name, rest) = repl.store.car_cdr(args)?;
let (vars, rest) = repl.store.car_cdr(&rest)?;
let (body, props) = repl.store.car_cdr(&rest)?;
let (name, rest) = repl.store.car_cdr_simple(args)?;
let (vars, rest) = repl.store.car_cdr_simple(&rest)?;
let (body, props) = repl.store.car_cdr_simple(&rest)?;
if !name.is_sym() {
bail!(
"Protocol name must be a symbol. Got {}",
Expand Down Expand Up @@ -807,9 +810,9 @@ where
.with_context(|| "evaluating protocol")?;
let ptcl = &io[0];

let (fun, rest) = repl.store.car_cdr(ptcl)?;
let (fun, rest) = repl.store.car_cdr_simple(ptcl)?;

let (car, rest) = repl.store.car_cdr(&rest)?;
let (car, rest) = repl.store.car_cdr_simple(&rest)?;
let Some(backend) = repl.store.fetch_string(&car) else {
bail!("Backend must be a string")
};
Expand All @@ -819,7 +822,7 @@ where
_ => bail!("Invalid value for backend"),
};

let (car, _) = repl.store.car_cdr(&rest)?;
let (car, _) = repl.store.car_cdr_simple(&rest)?;
let (Tag::Expr(ExprTag::Num), RawPtr::Atom(rc_idx)) = car.parts() else {
bail!("Reduction count must be a Num")
};
Expand Down Expand Up @@ -945,8 +948,8 @@ where
" '(13 . 17))",
],
run: |repl, args, _path| {
let (ptcl, rest) = repl.store.car_cdr(args)?;
let (path, args) = repl.store.car_cdr(&rest)?;
let (ptcl, rest) = repl.store.car_cdr_simple(args)?;
let (path, args) = repl.store.car_cdr_simple(&rest)?;

let path = get_path(repl, &path)?;

Expand Down
8 changes: 4 additions & 4 deletions src/cli/repl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,16 @@ impl<F: LurkField, C: Coprocessor<F> + Serialize + DeserializeOwned> Repl<F, C>
}

fn peek1(&self, args: &Ptr) -> Result<Ptr> {
let (first, rest) = self.store.car_cdr(args)?;
let (first, rest) = self.store.car_cdr_simple(args)?;
if !rest.is_nil() {
bail!("At most one argument is accepted")
}
Ok(first)
}

fn peek2(&self, args: &Ptr) -> Result<(Ptr, Ptr)> {
let (first, rest) = self.store.car_cdr(args)?;
let (second, rest) = self.store.car_cdr(&rest)?;
let (first, rest) = self.store.car_cdr_simple(args)?;
let (second, rest) = self.store.car_cdr_simple(&rest)?;
if !rest.is_nil() {
bail!("At most two arguments are accepted")
}
Expand Down Expand Up @@ -588,7 +588,7 @@ where
}

fn handle_meta(&mut self, expr_ptr: Ptr, file_path: &Utf8Path) -> Result<()> {
let (car, cdr) = self.store.car_cdr(&expr_ptr)?;
let (car, cdr) = self.store.car_cdr_simple(&expr_ptr)?;
match &self.store.fetch_sym(&car) {
Some(symbol) => {
let cmdstr = symbol.name()?;
Expand Down
17 changes: 8 additions & 9 deletions src/cli/zstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,14 @@ impl<F: LurkField> ZDag<F> {
intern_ptrs_hydrated!(store, *z_ptr.tag(), *z_ptr, ptr1, ptr2, ptr3, ptr4)
}
Some(ZPtrType::Env(sym, val, env)) => {
let sym = self.populate_store(sym, store, cache)?;
let val = self.populate_store(val, store, cache)?;
let env = self.populate_store(env, store, cache)?;
let raw = store.intern_raw_ptrs([
*sym.raw(),
store.tag(*val.tag()),
*val.raw(),
*env.raw(),
]);
let (_, sym_raw) = self.populate_store(sym, store, cache)?.into_parts();
let (val_tag, val_raw) =
self.populate_store(val, store, cache)?.into_parts();
let (_, env_raw) = self.populate_store(env, store, cache)?.into_parts();
let raw = store.intern_raw_ptrs_hydrated(
[sym_raw, store.tag(val_tag), val_raw, env_raw],
FWrap(*z_ptr.value()),
);
Ptr::new(Tag::Expr(Env), raw)
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/coprocessor/gadgets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ pub(crate) fn deconstruct_env<F: LurkField, CS: ConstraintSystem<F>>(
let env_ptr = s.to_ptr(&env_zptr);

let (a, b, c, d) = {
if let Some([v, val, new_env]) = s.pop_binding(env_ptr) {
if let Some([v, val, new_env]) = s.pop_binding(&env_ptr) {
let v_zptr = s.hash_ptr(&v);
let val_zptr = s.hash_ptr(&val);
let new_env_zptr = s.hash_ptr(&new_env);
Expand Down
2 changes: 1 addition & 1 deletion src/coroutine/memoset/demo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<F: LurkField> Query<F> for DemoQuery<F> {
}

fn from_ptr(_: &Self::RD, s: &Store<F>, ptr: &Ptr) -> Option<Self> {
let (head, body) = s.car_cdr(ptr).expect("query should be cons");
let (head, body) = s.car_cdr_simple(ptr).expect("query should be cons");
let sym = s.fetch_sym(&head).expect("head should be sym");

if sym == Symbol::sym(&["lurk", "user", "factorial"]) {
Expand Down
6 changes: 3 additions & 3 deletions src/coroutine/memoset/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl<F: LurkField> Query<F> for EnvQuery<F> {
let s = scope.store.as_ref();
match self {
Self::Lookup(var, env) => {
if let Some([v, val, new_env]) = s.pop_binding(*env) {
if let Some([v, val, new_env]) = s.pop_binding(env) {
if s.ptr_eq(var, &v) {
let t = s.intern_t();
s.cons(val, t)
Expand All @@ -57,11 +57,11 @@ impl<F: LurkField> Query<F> for EnvQuery<F> {
}

fn from_ptr(_: &Self::RD, s: &Store<F>, ptr: &Ptr) -> Option<Self> {
let (head, body) = s.car_cdr(ptr).expect("query should be cons");
let (head, body) = s.car_cdr_simple(ptr).expect("query should be cons");
let sym = s.fetch_sym(&head).expect("head should be sym");

if sym == Symbol::sym(&["lurk", "env", "lookup"]) {
let (var, env) = s.car_cdr(&body).expect("query body should be cons");
let (var, env) = s.car_cdr_simple(&body).expect("query body should be cons");
Some(Self::Lookup(var, env))
} else {
None
Expand Down
16 changes: 8 additions & 8 deletions src/coroutine/memoset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,9 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
Ok(Provenance::new(query, *result, dependencies))
}

fn provenance_from_kv(&self, kv: Ptr) -> Result<Provenance, MemoSetError> {
fn provenance_from_kv(&self, kv: &Ptr) -> Result<Provenance, MemoSetError> {
let store = self.store.as_ref();
let (query, result) = store.car_cdr(&kv).expect("kv missing");
let (query, result) = store.car_cdr_simple(kv).expect("kv missing");
let query_dependencies = self.dependencies.get(&query);

let dependencies = if let Some(deps) = query_dependencies {
Expand All @@ -401,7 +401,7 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
let s = self.store.as_ref();
let mut memoset = s.num(F::ZERO);
for kv in self.toplevel_insertions.iter() {
let provenance = self.provenance_from_kv(*kv).unwrap();
let provenance = self.provenance_from_kv(kv).unwrap();
memoset = self.memoset.acc_add(&memoset, provenance.to_ptr(s), s);
}
memoset
Expand All @@ -411,7 +411,7 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
let s = self.store.as_ref();
let mut transcript = Transcript::new(s);
for kv in self.toplevel_insertions.iter() {
let provenance = self.provenance_from_kv(*kv).unwrap();
let provenance = self.provenance_from_kv(kv).unwrap();
transcript.add(s, *provenance.to_ptr(s));
}
transcript.acc
Expand Down Expand Up @@ -754,7 +754,7 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
let mut unique_keys: IndexMap<usize, Vec<Ptr>> = Default::default();

let mut record_kv = |kv: &Ptr| {
let key = s.car_cdr(kv).unwrap().0;
let key = s.car_cdr_simple(kv).unwrap().0;
let kv1 = kvs_by_key.entry(key).or_insert_with(|| {
let index = Q::from_ptr(&self.runtime_data, s, &key)
.expect("bad query")
Expand Down Expand Up @@ -785,7 +785,7 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
});

for kv in self.toplevel_insertions.iter() {
let provenance = self.provenance_from_kv(*kv).unwrap();
let provenance = self.provenance_from_kv(kv).unwrap();
transcript.add(s, *provenance.to_ptr(s));
}

Expand All @@ -804,7 +804,7 @@ impl<F: LurkField, Q: Query<F>> Scope<Q, LogMemo<F>, F> {
// `unique_keys`.
let kv = kvs_by_key.get(key).expect("kv missing");
let count = self.memoset.count(kv);
let provenance = self.provenance_from_kv(*kv).unwrap();
let provenance = self.provenance_from_kv(kv).unwrap();
(provenance, count)
} else {
(Provenance::dummy(s), 0)
Expand Down Expand Up @@ -1171,7 +1171,7 @@ impl<F: LurkField, RD> CircuitScope<F, LogMemoCircuit<F>, RD> {
for (i, kv) in scope.toplevel_insertions.iter().enumerate() {
let s = scope.store.as_ref();
let cs = ns!(cs, format!("toplevel_insertion-{i}"));
let (key, value) = s.car_cdr(kv).expect("kv missing");
let (key, value) = s.car_cdr_simple(kv).expect("kv missing");
// NOTE: This is an unconstrained allocation, but when actually hooked up to Lurk reduction, it must be
// constrained appropriately.
let allocated_key =
Expand Down
2 changes: 1 addition & 1 deletion src/lem/coroutine/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ fn run<F: LurkField>(
let img_ptr = bindings.get_ptr(img)?;
let preimg_ptrs = scope
.store
.pop_binding(img_ptr)
.pop_binding(&img_ptr)
.context("cannot extract {img}'s binding")?;
for (var, ptr) in preimg.iter().zip(preimg_ptrs.iter()) {
bindings.insert_ptr(var.clone(), *ptr);
Expand Down
4 changes: 2 additions & 2 deletions src/lem/coroutine/toplevel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,15 @@ impl<F: LurkField> Query<F> for ToplevelQuery<F> {
ToplevelCircuitQuery { name, args }
}
fn from_ptr(toplevel: &Arc<Toplevel<F>>, s: &Store<F>, ptr: &Ptr) -> Option<Self> {
let (head, mut acc) = s.car_cdr(ptr).expect("query should be cons");
let (head, mut acc) = s.car_cdr_simple(ptr).expect("query should be cons");
let name = s.fetch_sym(&head).expect("head should be sym");
let num_args = toplevel.get(&name).unwrap().func.input_params.len();
assert!(num_args > 0, "cannot yet make 0 argument queries");
let mut args = vec![];
let _p = PhantomData;
if !acc.is_nil() {
while args.len() < num_args - 1 {
let (arg, rest) = s.car_cdr(&acc).expect("can't find image for cons");
let (arg, rest) = s.car_cdr_simple(&acc).expect("can't find image for cons");
args.push(arg);
acc = rest;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lem/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl Block {
Op::PopBinding(preimg, img) => {
let img_ptr = bindings.get_ptr(img)?;
let preimg_ptrs = store
.pop_binding(img_ptr)
.pop_binding(&img_ptr)
.context("cannot extract {img}'s binding")?;
for (var, ptr) in preimg.iter().zip(preimg_ptrs.iter()) {
bindings.insert_ptr(var.clone(), *ptr);
Expand Down
6 changes: 6 additions & 0 deletions src/lem/pointers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ impl Ptr {
(tag, raw)
}

#[inline]
pub fn into_parts(self) -> (Tag, RawPtr) {
let Ptr { tag, raw } = self;
(tag, raw)
}

#[inline]
pub fn has_tag(&self, tag: &Tag) -> bool {
self.tag() == tag
Expand Down
Loading

1 comment on commit 7a2f3b2

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Benchmarks

Table of Contents

Overview

This benchmark report shows the Fibonacci GPU benchmark.
NVIDIA L4
Intel(R) Xeon(R) CPU @ 2.20GHz
32 vCPUs
125 GB RAM
Workflow run: https://github.com/lurk-lab/lurk-rs/actions/runs/8239478393

Benchmark Results

LEM Fibonacci Prove - rc = 100

ref=770b55d902d4ed5c5a18d7c43643135a45d0fa62 ref=7a2f3b2c096e6bb20e054ec797ef3f7632e168ca
num-100 1.45 s (✅ 1.00x) 1.45 s (✅ 1.00x slower)
num-200 2.77 s (✅ 1.00x) 2.77 s (✅ 1.00x slower)

LEM Fibonacci Prove - rc = 600

ref=770b55d902d4ed5c5a18d7c43643135a45d0fa62 ref=7a2f3b2c096e6bb20e054ec797ef3f7632e168ca
num-100 1.84 s (✅ 1.00x) 1.85 s (✅ 1.00x slower)
num-200 3.05 s (✅ 1.00x) 3.06 s (✅ 1.00x slower)

Made with criterion-table

Please sign in to comment.