Explicitly copy token sequences to avoid sharing input

[EggBaconAndSpam]

Closes #492

As mentioned in the issue, we can avoid leaking memory retaining the input in memory by explicitly copying token sequences as we read them.

The performance implications of adding an explicit copy to takeN_ and takeWhile_ are as follows:

• The takeWhileP family of basic combinators end up consuming a lot more memory (since previously they didn't need to allocate the result buffer) and time (due to the copy call).
• The decimal (octal etc) family of combinators are about 40% slower and consume about twice as much memory.
• All other combinators (like many) have either stayed the same or got slightly (up to 10%) faster.

Sadly we don't have any real-world benchmarks (or do we?) so it's hard to tell if this would have any significant impact in practice.

Raw benchmark results

This PR: memory speed
Master: memory speed

[mrkkrp]

The way forward here is to first bring up to date the nix stuff that allows us to observe effects of such changes on dependent packages. I will do it soon. Once that is done we will be able to make an informed decision.

[mrkkrp]

Here are the results of benchmark before (old) and after (new) the changes for 3 packages (parsers-bench from this repo, mmark, and modern-uri):

• new-mmark-bench-memory.txt
• new-mmark-bench-speed.txt
• new-modern-uri-bench-memory.txt
• new-modern-uri-bench-speed.txt
• new-parsers-bench-memory.txt
• new-parsers-bench-speed.txt
• old-mmark-bench-memory.txt
• old-mmark-bench-speed.txt
• old-modern-uri-bench-memory.txt
• old-modern-uri-bench-speed.txt
• old-parsers-bench-memory.txt
• old-parsers-bench-speed.txt

While the impact is sometimes negligible, in certain cases it is quite noticeable (notably in the parsers from parsers-bench). Perhaps we should give the users a choice whether to perform copying and avoid sharing?

[EggBaconAndSpam]

Thanks for sorting out the benchmarks! I had completely overlooked that parsers-bench is part of this repo as well 😅

I agree the impact appears to be large enough that we probably should give people a choice.

I propose adding two newtype wrappers ShareInput and NoShareInput (names are up for debate 😄) to explicitly select one or the other Stream implementation. The unwrapped instances should coincide with their ShareInput counterparts at least for now, i.e. instance Stream a = instance Stream (ShareInput a) in handwavy terms.

In a future major release we could change the default behaviour to NoShareInput, with a short note on what input sharing is and to try ShareInput if performance is a concern.

What do you think? Shall I update the PR along those lines?

[mrkkrp]

This sounds good, please go ahead.

[EggBaconAndSpam]

I added ShareInput and NoShareInput. What does this look like to you? 🙂

[mrkkrp]

Thanks!