GregoryConrad / rearch-rs

Re-imagined approach to application design and architecture
https://crates.io/crates/rearch
MIT License
81 stars 4 forks source link

Consider `parking_lot`'s `RwLock` instead of concread #44

Closed GregoryConrad closed 5 months ago

GregoryConrad commented 5 months ago

Currently, read/write locks are only held for a very short amount of time. I originally choose concread because it supports concurrent writers/readers (so reads always remain unblocked), but I'm not sure it actually helps our specific benchmarks. One example is when a capsule is not initialized, you will need to grab a write, which is blocking.

The specific issue I'm facing with concread is that I can't support a Container::read_ref api with it right now. I'd need the ability to commit a write txn, and in that process, downgrade it to a read txn. I may file an issue over at concread to see if they'd support this, but if parking_lot's RwLock around a standard HashMap is faster anyways, I'm not sure it'd be worth it, as they support downgradable writes.

GregoryConrad commented 5 months ago

Also should consider:

GregoryConrad commented 5 months ago

Based on the count benchmark in /examples (which isn't scientific by any stretch of the imagination), I got the following results. Perfect? No. Good enough to show trends and make architectural decisions? Probably, yes.

chart-2 chart

As you may predict, concread supports a large number of readers better. However, the RwLock with HashMap variant is faster for a smaller number of readers, and is always faster for writers. So the proper choice here is likely to depend upon the expected workload.

I'll sleep on this and see what I think later.

GregoryConrad commented 5 months ago

I am originally inclined to say that I am tempted to ditch concread, because:

GregoryConrad commented 5 months ago

Also, if we do go down the no concread route, we will want to add in the container.read_ref method

GregoryConrad commented 5 months ago

Going to remove concread. Need to still add the Container::read_ref api.