byteverse / contiguous

Typeclass for array types
Other
19 stars 9 forks source link

Missing `unsafeThaw` #66

Open Qqwy opened 4 weeks ago

Qqwy commented 4 weeks ago

I am using Contiguous to implement a new immutable HAMT-style hashmap. As part of this, I want to provide an unsafeInsert (which is a building block to make other construction functions, like fromList fast). This, in turn, is built upon unsafeThaw, to mutate the existing immutable arrays in place.

unsafeThaw is supported by the various arrays Contiguous is wrapping, but not yet part of the class.

Would a PR adding it be accepted?

andrewthad commented 4 weeks ago

Although unsafeThaw exists as a primop in GHC, it is extremely dangerous. I've only ever seen it used in unordered-containers to help write fromList, and even there, I'm skeptical of its soundness.

It's not possibly to make fromList faster by mutating an immutable array. The existing implementation mutates a mutable array and then freezes it (in place) at the end. Can you describe something in more detail that would benefit from unsafeThaw? Maybe you do have a way to use unsafeInsert to make a faster fromList, but I am not able to see how it could possibly help.

andrewthad commented 4 weeks ago

Sorry, I just realized that I missed the first sentence that you wrote. I assume that you're copying the design of unordered-containers. I'll take a PR for unsafeThaw. I think it should only be available for ContiguousU types though, not all Countiguous types.