Support thread-safe program execution

[inikolaev]

I'm experimenting with writing a Python extension for this library using pyo3 and running into issues when it comes down to concurrency. I'm not really well-versed in Rust, but I asked about it here. As far as I understand it boils down to using Arc instead of Rc.

I'm currently using a Python version of CEL interpreter, but its performance leaves a lot to be desired, so I'm looking for an alternatives. I use CEL for feature flags so we have multiple compiled expressions which are evaluated from different threads.

What are your thoughts about it? What would it take to make the interpreter thread-safe?

I'm willing to help, but my Rust knowledge if very very limited :)

[inikolaev]

I have actually try replacing all Rc into Arc and it seems to be working and all tests seem to pass, so at the very least this is a working solution, but I don't know if there are any drawbacks.

[clarkmcc]

Hi! Good idea, so I think the program itself (not the context) needs to be threadsafe (which means converting Rcs to Arcs as you say) and then you would pass a separate context to each execution concurrently. I did the conversion and ran some performance benchmarks and we're looking at <5% regression on affected benchmarks, but we're also looking at +5% performance improvement on some other benchmarks which doesn't make a whole lot of sense to me so I suspect that the margin of error is at least a few percentage points.

I opened a PR for this. Let me take some time to think over this and make sure it all makes sense.

In the meantime, you should be able to reference the branch specifically in your Cargo.toml dependencies file.

criterion.zip

[clarkmcc]

Okay so after more thought, it makes more sense to do Arc<str> instead of Arc<String> since the strings are immutable. This allows us to avoid a double indirection. I'm working on the port right now and am working through two primary concerns:

1. The developer experience of the change -- what is it like to work with Arc<str> as opposed to Arc<String>
2. The performance comparison -- a simple find-and-replace with Arc<str> actually has horrible benchmark stats (50-100% slower) which doesn't make conceptual sense so I'm working through each case and finding clones that don't need to happen anymore, etc.

Anyways, that's the status update.

[clarkmcc]

Ran some benchmarks, did some profiling, and asked around online, and it sounds like there are cases where Arc<str> is less performant then Arc<String>, specifically depending on how it's allocated. I think we could squeeze some more performance out of Arc<str> but not without a lot of gutting. Given that the performance decrease from Rc to Arc is between 1-7%, and given that we're orders of magnitude faster than the Go version of this library, I'm good with Arc<String>. I'll get a PR finished and merged shortly.
criterion.zip