On Tue, Mar 27, 2018 at 03:42:32AM +0800, Xiao Guangrong wrote: [...] > > It'll be understandable to me if the problem is that the compress() > > API does not allow the input buffer to be changed during the whole > > period of the call. If that is a must, this patch for sure helps. > > Yes, that is exactly what i want to say. :) So I think now I know what this patch is for. :) And yeah, it makes sense. Though another question would be: if the buffer is updated during compress() and compress() returned error, would that pollute the whole z_stream or it only fails the compress() call? (Same question applies to decompress().) If it's only a compress() error and it won't pollute z_stream (or say, it can be recovered after a deflateReset() and then we can continue to call deflate() without problem), then we'll actually have two alternatives to solve this "buffer update" issue: 1. Use the approach of current patch: we copy the page every time, so deflate() never fails because update never happens. But it's slow since we copy the pages every time. 2. Use the old approach, and when compress() fail, we just ignore that page (since now we know that error _must_ be caused by page update, then we are 100% sure that we'll send that page again so it'll be perfectly fine). If you see, IMHO method 2 has its advantage, since actually it "detects" the page update operation by getting a failure in compress(), then we don't really need to send that page at all (since we'll send it later again, for sure). Then, we not only saved the memcpy() CPU time for every single page, meanwhile we might save some bandwidth since we won't bother to send the page when we know the page is modified. But all these depend on the assumption that: 1. compress() will fail only because of buffer update, and 2. compress() failures won't pollute the whole z_stream. Same thing would apply to decompress() side - we drop the corrupted page (when decompress() returned errors) since we know another one will come soon. It's a bit tricky, but I'm still curious about it, since actually that's mostly the old code before this patch except that we don't really drop corrputed pages but we still use them (which won't hurt too IMHO). Thanks, -- Peter Xu
