fragile icon indicating copy to clipboard operation
fragile copied to clipboard

Use `std::thread::ThreadId` instead of custom thread id

Open Noratrieb opened this issue 1 year ago • 7 comments

std guarantees them to be unique, allowing us to use them here instead of having to roll our own thread ids.

Noratrieb avatar Oct 17 '22 14:10 Noratrieb

This undoes the niche optimization: https://github.com/mitsuhiko/fragile/pull/18

mitsuhiko avatar Oct 17 '22 15:10 mitsuhiko

ThreadId uses a NonZeroU64 as well, keeping the niche

Noratrieb avatar Oct 17 '22 15:10 Noratrieb

It does have the disadvantage of being a little bigger on 32 bit though.

Noratrieb avatar Oct 17 '22 15:10 Noratrieb

Oh I missed that. I will reopen this for now but I'm not sure yet what the advantage of reusing the threadid from the stdlib is.

mitsuhiko avatar Oct 17 '22 15:10 mitsuhiko

It makes the code a little simpler and avoids the need for having a custom thread_id module. But since it's a pretty simple module there isn't a huge benefit.

Noratrieb avatar Oct 17 '22 15:10 Noratrieb

Do you think we should do this? I'd resolve the conflicts then. If not, I can close it

Noratrieb avatar Nov 01 '22 14:11 Noratrieb

I’m probably in favor of not applying this. It makes the type bigger and I’m not convinced (yet at least) that using the thread ID from the stdlib is necessary. No need to resolve the conflicts but I will leave this open in case my opinion on this changes / gets changed :)

mitsuhiko avatar Nov 01 '22 17:11 mitsuhiko