Skip to content

Commit

Permalink
chore: some minor improvements
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 committed Mar 11, 2024
1 parent 770b55d commit 1365c75
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 120 deletions.
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
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
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
4 changes: 2 additions & 2 deletions src/coroutine/memoset/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
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

0 comments on commit 1365c75

Please sign in to comment.