Use unsafeWithForeignPtr where possible #372
Conversation
In GHC 9.0.1 `withForeignPtr` became (necessarily) considerably more expensive. In GHC ticket #19474, it was noted that `Storable` `Vector`s are affected by this regression. Fix this by using `unsafeWithForeignPtr` when it is known that the continuation is not divergent. See also GHC #17760.
lehins
left a comment
There was a problem hiding this comment.
@bgamari I didn't realize you finished keepAlive# for 9.0.1 Thank you for doing the work and for submitting this PR.
I think we also need to use unsafeWithForeignPtr in:
Data.Vector.Storable.unsafeWithData.Vector.Storable.Mutable.unsafeWith
They both are already unsafe, so we just need to add a note to documentation that the supplied action must terminate.
@bgamari Do you think you can add it to this PR? I can do it later myself of course, unless you have an argument against using unsafeWithForeignPtr in those functions.
Yes, I saw these but I'll admit I was a bit hesitant to extend the new behavior to |
|
LGTM. This looks quite bad so I think this should backported to 0.12 branch after PR is merged. |
100% agree. |
|
I'm curious: Could this perf bug have been caught be comparing |
|
@sjakobi several benchmarks got 5-6% slower, but this is not that much alarming on its own. |
In GHC 9.0.1
withForeignPtrbecame (necessarily) considerably moreexpensive. In GHC ticket #19474, it was noted that
StorableVectorsare affected by this regression. Fix this by using
unsafeWithForeignPtrwhen it is known that the continuation isnot divergent.
See also GHC #17760.