Closed GregoryConrad closed 9 months ago
I’ve put some thought to it, and the opt-in approach really feels like a hack on the implementation side, but manual txns can be less ergonomic in many scenarios.
Alas, here are some links that may be handy if we go with the opt-in approach:
Idea is that we have an atomic Option
I may try to implement both approaches to see how each looks/feels from both an API and implementation perspective and pick based on that.
Got a good chunk of implementation work done on the side-effect-txns
branch for the explicit txn approach (like container.run(|txn| set_state(txn, 1234));
). Probably won't finish the implementation until I'm sure I want to go down that road because it'll take a good chunk of time.
Explicit transaction approach:
Jokes aside, the implementation is fairly clean and easy, which is a good sign.
Issue is that it is atrocious to work with. Aside from manually passing around txn
everywhere, Rust has a really annoying bug in closures that follow the form |foo: &Foo| -> &Bar {foo.bar}
because the lifetimes aren't properly related. And of course, explicitly writing out the lifetimes to fix this requires nightly. The other workaround that I used in my branch is a function that helps the compiler out with lifetimes, but forcing users into a workaround like that is 🫤
I am going to try to figure out the non-explicit (like rearch-dart) way next, if it is even possible. Frankly not sure, but I'll give it a go. I will likely need a CapsuleRebuilder
like structure in a thread-local variable of some sort.
For the dart-like approach:
Was originally going with thread_local: this is wrong because each container needs its own independent transactions (versus just a thread).
also, I was a bit stumped with initial impl since locking a mutex in the same thread isn’t allowed (possible panic or deadlock). I actually learned about the “reentrant mutex” in this process and that should do the trick when put in the ContainerStore (ReentrantMutex<RefCell<HashSet
we can keep the write txn within its own normal mutex. Since the reentrant mutex will lock the entire txn process, only one thread at a time will access the data/nodes mutex. Also, continued locks of those mutexes in the same thread should be quick since there won’t be any contention and they should pick it up instantly since it was last held by the same thread.
Pushed up some kinda-working code (side effects broken) for the Dart-like transactions into dart-like-side-effect-txn
. I like this approach to the side effect transactions more than the explicit approach, so I will likely push this one to completion and see how it goes.
In case the current side effect txn model, for whatever reason, needs to be changed, see the below patch off of commit 92ed98c92d70e45d1257f66cfb271b0098381697 that starts the implementation of explicit side effect transactions.
I can think of two ways to implement this.
txn_modified_capsules: Option<HashSet<Id>>
, where iftxn_modified_capsules
isSome
, no rebuilds will take place until it is committed (i.e. they are ignored), or something like that. IfNone
, uses the normal rebuild on each call behavior. Need some more thought here. Note that this must be in a transaction or something behind a Mutex, because this is not possible to do in parallel safely (competing transactions would wreak havoc).container.run(increment_count)
and completely remove theCapsuleRebuilder
structure. Likecontainer.run(|txn| { increment_count(txn); set_state(txn, 1234); });