DEV Community

Cover image for PR#1: Make SurrealDB performance slightly better
Hossein Mobarakian
Hossein Mobarakian

Posted on

PR#1: Make SurrealDB performance slightly better

At the first step, I picked up the SurrealDB project for contribution. I didn't know how I could help this project become better. So I asked my beautiful OpenCode to find parts of the project that could be better.

It suggested this file of the project(core/src/val/value/get.rs) to me and said it has a double-cloning issue.

So I opened up VS Code, and I started checking the issue. The code was something like this:

let mut a = Vec::new();
for v in v.iter() {
    let cur = v.clone().into();
    if stk
         .run(|stk| w.compute(stk, ctx, opt, Some(&cur)))
         .await
             .catch_return()?.is_truthy()
         {
            a.push(v.clone()); 
         }
}
Enter fullscreen mode Exit fullscreen mode

First Optimization:

As you can see at line 3 and line 9, we have multiple clones from a single document. I thought about how I could fix this issue; I went to see the CursorDoc structure because the first clone is converted to it:

#[derive(Clone, Debug)]
pub(crate) struct CursorDoc {
    pub(crate) rid: Option<Arc<RecordId>>,
    pub(crate) ir: Option<Arc<IteratorRecord>>,
    pub(crate) doc: CursorRecord,
    pub(crate) fields_computed: bool,
}

impl From<Value> for CursorDoc {
    fn from(val: Value) -> Self {
        Self {
            rid: None,
            ir: None,
            doc: val.into(),
            fields_computed: false,
        }
    }
}

#[derive(Clone, Debug)]
pub(crate) struct CursorRecord {
    /// The underlying record, shared via Arc for copy-on-write
    record: Arc<Record>,
}

impl CursorRecord {
        // .... //
    /// cloning. Otherwise the value is cloned.
    pub(crate) fn into_owned(self) -> Value {
        match Arc::try_unwrap(self.record) {
            Ok(record) => record.data,
            Err(arc) => arc.data.clone(),
        }
    }
        // .... //
}

impl From<Value> for CursorRecord {
    fn from(value: Value) -> Self {
        Self {
            record: Arc::new(Record::new(value)),
        }
    }
}

Enter fullscreen mode Exit fullscreen mode

I saw that the value passed through CursorDoc is directly stored in a field in CursorRecord without any changes, and it is accessible using .into_owned() from CursorRecord.

That is the solution; I edited the code according to it:

for v in v.iter() {
    let owned_v = v.clone();
    let cur = CursorDoc::from(owned_v);
    if stk
        .run(|stk| w.compute(stk, ctx, opt, Some(&cur)))
        .await
        .catch_return()?
        .is_truthy()
    {
        a.push(cur.doc.into_owned()); 
    }
}

Enter fullscreen mode Exit fullscreen mode

Now we have only a clone of the document without any overhead.

Second Optimization:

As you can see we have for loop with .await; it is a performance issue because awaits are executing sequential. If we have 200 items in the loop and each takes 10ms to execute, it will take 2 seconds!

To prevent this, I made it parallel using the same structure that exists in SurrealDB code:


let a: Vec<Value> = stk.scope(|scope| {
  let futs = v.iter().map(|v| {
    scope.run(|stk| {
       let cur = CursorDoc::from(v.clone());
        async move {
          let res = match w.compute(stk, ctx, opt, Some(&cur)).await.catch_return(){
                    Ok(v) => v,
                    Err(e)=> {
                        return Err(e);
                    }
                };

                if res.is_truthy() {
                    Ok(Some(cur.doc.into_owned()))
                } else {
                    Ok(None)
                }
            }
        })
    });
    try_join_all_buffered(futs, ctx.config.max_concurrent_tasks)
})
.await?
.into_iter()
.flatten()
.collect();
Enter fullscreen mode Exit fullscreen mode

Using stk.scope and try_join_all_buffered, I execute mapped futures and send the response to "a" Vector after finishing.

Now, for complex queries, it will work much faster by leveraging the multi-thread principle.

Hope the SurrealDB team will merge this PR😁

Top comments (0)