serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@gmail.com>
Subject Re: limited readline (was: svn commit: r1713489 - serf_bucket_readline())
Date Wed, 11 Nov 2015 16:15:27 GMT
And I see you built this in '788. (catching up here)


On Wed, Nov 11, 2015 at 10:05 AM, Greg Stein <gstein@gmail.com> wrote:

> On Tue, Nov 10, 2015 at 4:44 PM, Bert Huijben <bert@qqmail.nl> wrote:
> >...
>
>> There is one more problem with all this wrapping: All layers need to
>> somehow support all read methods.
>>
>> Wrapping works for all methods except: readline.
>>
>
> If all methods are passed to the wrapped bucket, then readline works fine.
>
>
>> This method is only implemented on a few bucket types, and doesn't allow
>> passing an upper limit to whatever is read... so things like limit and
>> unframing buckets can never implement this without implementing some
>> caching...
>>
>> And once they introduce caching all read methods need to look at the
>> cache.
>>
>
> Sure. That's what 'databuf' is all about. To implement that cache in a
> general way.
>
> Does it play nice with "limit" type buckets? Well, no... it overreads.
> Those buckets will need to be *much* smarter.
>
> I believe the correct answer is that limit-type buckets need to peek
> first, look for CR and/or LF, and then read() that much to return the
> caller. Limited, as appropriate.
>
> Because each read() will be preceded by a peek(), I think adding a
> peek_iovecs() can improve the performance. It can possibly provide more
> data to find that CR/LF. It's not required, but it might be a nice addition.
>
> I disagree with the recently-added readline2() because it's use is *so*
> minimal. It would be unfortunate to have two entry points with such similar
> signatures.
>
> We could add a utility function: serf_bucket_limited_readline() that is a
> cover over peek/readline.
>
> This problem is not that relevant for writing requests, as that reads raw
>> data anyway... but we are having more and more buckets that just set the
>> readline implementation to NULL.
>>
>
> Well, that was just them being lazy :-P ... I see you're fixing some of
> those buckets.
>
> Cheers,
> -g
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message