iteratee countBytes, modifier which counts 'bytes'
jwlato at gmail.com
Tue Jan 10 11:28:46 GMT 2012
Apologies for delay; just got back from traveling. Thanks very much
for submitting this. Further comments below.
On Fri, Jan 6, 2012 at 9:34 AM, Alex Lang <lang at tsurucapital.com> wrote:
> Here is an updated patch which addresses some of the issues raised by Michael.
>> 1) function name is very misleading, it should be called something
>> like countElements or countConsumed
> Indeed, I've changed the name to countConsumed for now.
>> 2) function ignores value passed inside EOF constructor which means it
>> can be controlled from Enumerator
> I am threading the EOF streams through so I don't believe this is an issue
The EOF handling looks correct to me, although it is slightly tricky
to reason about.
>> 3) I suspect that "newLen = n + fromIntegral (LL.length c)" will cause
>> memory leak because of lazyness.
> Indeed, fixed in the new patch
> Additionally, I've also changed the counter to be of generic Integral
> type rather than Int64. In private communication with Michael, he
> suggested that the type might as well be Integer, like some other
> functions in GHC, which index into files:
> hSeek :: Handle -> SeekMode -> Integer -> IO ()
> hFileSize :: Handle -> IO Integer
> This sounds reasonable to me.
Either way is fine with me; I've left it polymorphic for now. It's
nicely parallel to ListLike.genericLength. In fact, it may be better
to use LL.genericLength instead of (fromIntegral . LL.length), in case
the length of a chunk overflows an Int. That's a highly improbable
Thanks for also including a QuickCheck property; it's much appreciated.
I've INLINEd `countConsumed` and added a microbenchmark for it as
well. It's 2-3x faster than using ( `I.enumWith` I.length). I'll try
to improve `enumWith`, but it may not ever be as efficient as the
These changes are in the repo but not on Hackage yet. I need to add
some more tests for some other changes I made before I'm confident in
them; will try to get a hackage release out today.
More information about the Iteratee