hawkw / sharded-slab

a lock-free concurrent slab (experimental)
MIT License
273 stars 19 forks source link

fall back gracefully when max thread IDs are reached #62

Open hawkw opened 3 years ago

hawkw commented 3 years ago

When the thread ID limit defined by the Config is reached, we currently panic. This isn't great.

As I said in tokio-rs/tracing#1485:

I do think sharded-slab could handle this better. We could potentially reserve the last thread ID (in this case, 4096) for a special shard that allows concurrent mutation by multiple threads, with a mutex, and use that as a fallback when the thread ID cap is reached. That way, rather than panicking, we'd degrade to contending a mutex for threads over the max, which might be a better solution. However, that would be a bit of work to implement...but I'll open a ticket for it upstream.

danielhenrymantilla commented 3 years ago

I'm guessing this is the cause of a panic at the following line: https://github.com/hawkw/sharded-slab/blob/81c1c3fa846c364d9fcd6a7dde7841a7ce80f14b/src/shard.rs#L297 ?

If anything, replacing that [idx] with .get(idx).expect("too many threads") would already be quite helpful when debugging 🙂

hawkw commented 3 years ago

I'm guessing this is the cause of a panic at the following line: https://github.com/hawkw/sharded-slab/blob/81c1c3fa846c364d9fcd6a7dde7841a7ce80f14b/src/shard.rs#L297 ?

If anything, replacing that [idx] with .get(idx).expect("too many threads") would already be quite helpful when debugging 🙂

Yeah, that's a good point; we should at least make the panic message a bit more helpful in that case (and perhaps include what the configured thread count is).

I'd happily merge a PR that makes this change, if you're interested in opening one? Otherwise, I'll try to improve it when I get the chance.