New API to convert directly between lazy and short bytestrings #619
Conversation
vdukhovni
force-pushed
the
lazy-from-to-short
branch
from
0bbd297
to
0158b8c
Compare
| -> ByteString -- ^ initial strict chunk | ||
| -> LBS.ByteString -- ^ remaining chunks | ||
| -> IO (MutableByteArray RealWorld) -- ^ possibly resized result | ||
| lazyToShortIO mba used size c@(BS fptr len) cs |
There was a problem hiding this comment.
Would it be better not to pass size, but rather extract it every time with getSizeofMutableByteArray#? Admittedly this would require IO before any pattern-matching begins...
There was a problem hiding this comment.
This is a purely internal helper function, we could even move it into a where clause, to really make sure there are no unexpected calls. So I don't think such caution is warranted, I was much more concerned with handling integer arithmetic overflow, which I believe is handled correctly.
| -> LBS.ByteString -- ^ remaining chunks | ||
| -> IO (MutableByteArray RealWorld) -- ^ possibly resized result | ||
| lazyToShortIO mba used size c@(BS fptr len) cs | ||
| -- Safe against overflow since used <= size. |
There was a problem hiding this comment.
| -- Safe against overflow since used <= size. | |
| -- Safe against overflow since 0 <= used <= size. |
...or we could just say let newUsed = checkedAdd "Short.lazyToShort" used len. (This does have the marginal downside of checking for overflow in the no-new-buffer case.)
Another option is to allow used+len to signed-overflow, but check intToWord newUsed <= intToWord size since it cannot unsigned-overflow.
| -> lazyToShortIO mba (used + len) size c' cs' | ||
| -- Care to detect possible 'Int' arithmetic overflow. | ||
| | used < used + len | ||
| , let newsize = max (used + len) (size + size `quot` 2) |
There was a problem hiding this comment.
- If
(size + size `quot` 2)overflows and becomes negative, this will behave degenerately and cause O(n²) allocation for inputs that narrowly fit into theInt#size limit. - Signed
(`div` 2)will compile to an arithmetic right shift; signed(`quot` 2)needs an extra shift and add/subtract to fix up negative odd inputs. (Or(`shiftR` 1)makes the performance question much easier.)
No description provided.