Data.ByteString.Lazy.dropEnd: Use two-pointers technique #629
Conversation
This can be seen as using the input itself as an implicit queue; we formerly copied its chunks into an explicit queue. By writing the key logic as a polymorphic `splitAtEndFold`, it was easy to re-use it for `takeEnd`; the latter function should now operate in constant stack space and can release initial chunks of a very long input string sooner.
...so that a plain 'cabal test' finds the bug almost every try instead of finding it only every few dozen tries
(Some re-compilation check somewhere set a trap for me.) This also replaces fromIntegral with intToIndexTy in a few places.
Data/ByteString/Lazy.hs
Outdated
| where | ||
| -- Idea: Keep two references into the input ByteString: | ||
| -- "bsL" tracks the current split point, | ||
| -- "bsR" tracks the yet-unprocessed tail. |
There was a problem hiding this comment.
Is it true that bsR is a substring of bsL? Maybe calling them tortoise and hare as in Floyd's algorithm would be helpful.
There was a problem hiding this comment.
Yes. bsR is always a suffix of bsL, which is always a suffix of the original input. I'm not sure I like the suggested tortoise/hare naming here but agree bsL/bsR isn't great, either.
There was a problem hiding this comment.
bsL / bsR are quite misleading IMO: if I were to guess I'd say that they are "left bytestring" and "right bytestring" with regards to a certain split point such that bsL <> bsR equals the input string.
Even something very stupid like bs, subBs and subSubBs would be better.
|
I tried renaming to I'd like to verify with benchmarks that this performs as expected before I go ahead and merge it. |
Yes, it's better, thanks. Feel free to merge when you are happy with benchmarks. |
|
Baseline (df039bd): After (071a062): |
* Data.ByteString.Lazy.dropEnd: Use two-pointers technique This can be seen as using the input itself as an implicit queue; we formerly copied its chunks into an explicit queue. By writing the key logic as a polymorphic `splitAtEndFold`, it was easy to re-use it for `takeEnd`; the latter function should now operate in constant stack space and can release initial chunks of a very long input string sooner. * Fix a very silly bug, and strengthen tests ...so that a plain 'cabal test' finds the bug almost every try instead of finding it only every few dozen tries * Actually work around the poison instance (Some re-compilation check somewhere set a trap for me.) This also replaces fromIntegral with intToIndexTy in a few places. * Rewrite the poison instance using TypeError * Rename "bsL" -> "toSplit" and "bsR" -> "toScan" * Add basic benchmarks for lazy takeEnd/splitEnd According to these benchmarks, the new implementation for takeEnd is somewhat faster and the new implementation for dropEnd is roughly 3.5x to 4x as quick as its predecessor. (cherry picked from commit 2bbc97e)
This can be seen as using the input itself as an implicit queue; we formerly copied its chunks into an explicit queue.
By writing the key logic as a polymorphic
splitAtEndFold, it was easy to re-use it fortakeEnd; the latter function should now operate in constant stack space and can release initial chunks of a very long input string sooner.(I haven't attempted any benchmarking yet.)