zlib icon indicating copy to clipboard operation
zlib copied to clipboard

Allow to select flushing strategy `NoFlush` is the default, but the `…

Open tolysz opened this issue 8 years ago • 7 comments

Allow to select flushing strategy NoFlush is the default, but the SyncFlush is useful with the incremental protocols eg. compression with websockets

tolysz avatar Dec 23 '16 00:12 tolysz

@tolysz unfortunately this is in conflict with the API design layed out in #6 which would provide us with exact control over flushing which is something that streaming frameworks require (and the reason some can't use zlib currently and have to resort to their own bindings)

hvr avatar Dec 23 '16 08:12 hvr

;) I see no conflict. This change is independent of the other. I.e. it controls the default behavior. The other change (the proposed api change) will still be needed, as there is no explicit way flush buffer while building with NoFlush. In the real life, you need both as this library is useful in many possible protocols.

tolysz avatar Dec 23 '16 09:12 tolysz

@tolysz the conflict I see (unless I'm wrongly assuming what this PR does):

the new API would give you the choice to either flush explicitly (via compressFlush) or don't flush (when using compressSupplyInput);

      CompressInputRequired {
         compressFlush       :: m (CompressStream m),
          compressSupplyInput :: S.ByteString -> m (CompressStream m)
      }

but if you change the default, compressFlush would serve no purpose anymore, wouldn't it?

PS: or let me ask differently (as then I may understand better): what does #10 make possible to do we couldn't already do if #6 was already implemented?

hvr avatar Dec 23 '16 13:12 hvr

There will be applications which will require PUSH and PULL but you are right - not all combination for flush-strategy and explicit flushing will make sense.

The PR works and it should be transparent to most users. I am using it in implementation of permessage-deflate for WebSockets (formatRaw), thus I would need to call flush each time. PR will do it for me at the c level. Flush flags are already implemented in zlib just are in not exposed module, this change simply makes use of them. The alternative is to expose Streams from zlib and I could create zlib-flush library ;)

ps. I do not change the default, I allow the default to be changed.

tolysz avatar Dec 23 '16 13:12 tolysz

@tolysz are you sure this is what you really want? This would flush after every block, whereas even for websockets or whatever surely you'd want to flush at logical protocol boundaries rather than abitrary intermediate points depending on how the lazy bytestring was constructed.

I suspect that the stream interface with explicit flushing would work best for your use case, but I could be wrong. I'd be happy to get a more detailed explanation.

dcoutts avatar Feb 19 '17 10:02 dcoutts

@tolysz is this still in flight? I believe @dcoutts makes some good points here.

emilypi avatar Feb 19 '21 15:02 emilypi

It my still be useful, but in the end I switched to streaming-commons as I needed to implement it in the websockets library... and they already had it...

tolysz avatar Feb 19 '21 19:02 tolysz