You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Alan Carroll <so...@oath.com.INVALID> on 2018/04/19 18:20:58 UTC
Buffer Writer Formatting
I have several pull requests up currently involving updates to output
formatting for BufferWriter. I was asked to provide more detail on the
point of these pulls requests. Anyone who is interested can read this
document - https://solidwallofcode.github.io/buffer-writer.en.html for that
detail.
Re: Buffer Writer Formatting
Posted by Alan Carroll <so...@oath.com.INVALID>.
I'd be fine with doing this in my already scheduled "AMC Tech Corner" slot.
On Mon, Apr 23, 2018 at 11:22 AM, Bryan Call <bc...@apache.org> wrote:
> We should have a breakout session on this at the ATS Summit because it
> doesn’t look like this is going to be resolved on the mailing list. We can
> summarize the breakout session on the mailing list and open it up for
> anyone else that wants to add to the discussion.
>
> -Bryan
>
>
> > On Apr 20, 2018, at 7:00 PM, Alan Carroll <so...@oath.com.INVALID>
> wrote:
> >
> > I don't think Leif and Bryan are taking this seriously. For instance, I
> > went and checked the PRs dealing with BufferWriter formatting (3332). I
> > found one. In the same period, there was also a PR to deal with a
> compiler
> > problem with printf() (3476). One of these is dangerously unstable, the
> > other solid and reliable.
> >
> > The code is hardly pushing the compiler technology envelope - it's all
> > C++11 with one small utility class ported from C++14 (which uses only
> C++11
> > features). This is 7 year old technology, it only seems new because we're
> > so far behind.
> >
> > Similarly Bryan complains "On the design, formatting of objects shouldn't
> > be centralized. This creates dependency problems and problems with
> access
> > private members. Each object should have its own way of formatting in a
> > decentralized design." Which is exactly what the design does, lets each
> > type define its own formatting overload function. This isn't an
> incidental
> > feature, it's the core of the work.
> >
> > I do appreciate your concern for my time, but as the person who has (1)
> > done the work and (2) used the library, my view is I expect an overall
> time
> > saving from the endeavor. Frankly, just using it for IP addresses I would
> > consider a win. Even ignoring that, it's a sunk cost now, almost all the
> > work is done.
> >
> > P.S. So, if I say I don't want to replace printf, there's no problem? Of
> > course, no small part of the reason for the work on basic types like
> > integers was to get the performance level Leif and Bryan said was
> required.
> > When Bryan asked me to do the write up for the mailing list, I
> deliberately
> > took out any mention of Debug() in order to focus on the actual work, not
> > future plans. But that wasn't taken seriously either.
> >
> > On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> >> Traveling, so keeping this short for now...
> >>
> >>> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID>
> >> wrote:
> >>>
> >>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> >>> agree the "extra" value is small for TSDebug. I feel the use of
> >>> bufferwritter is great within our code base.
> >>
> >>
> >> Many of us have spent significant amounts of time dealing with how to
> deal
> >> with he integration of this library on various platforms. I have yet to
> see
> >> the benefits here outweigh the work efforts. I have little confidence
> that
> >> the code would not continue to be a PITA when dealing with cross
> platforms
> >> / compilers. It’s just really pushing the envelope on how compilers
> >> implement these some of these features.
> >>
> >> The other concern I have is that we might be building a library that is
> to
> >> difficult to use,that people wont bother and rather just printf things.
> I
> >> know you fixed the performance issues that were noticeable before, but
> this
> >> is still a complex piece of code that IMO has yet to show value.
> >>
> >> Yes, pretty harsh, and we should have had this discussion long ago. I
> also
> >> know that some of this work was done to deal with string expansion in
> >> header_rewrite; but this is definitely not what I had in mind at the
> time.
> >> I wanted to just to be able to efficiently expand the %{} conditions in
> >> strings, not implement printf as it is.
> >>
> >> Ciao,
> >>
> >> — Leif
> >>>
> >>> -Jason
> >>>
> >>>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
> >>>>
> >>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
> >>>> benefit. Also, I don’t think adding another formatting interface for
> >>>> strings is something we want to maintain or use.
> >>>>
> >>>> The main downside, with snprintf(), I see reading the examples is
> having
> >>>> to keep track of the length and position in the buffer if you are
> >> calling
> >>>> snprintf() multiple times. This can be handled writing a simple
> wrapper
> >>>> around snprintf(), which I have done before in about 20 lines of code.
> >> If
> >>>> we want to expose a wrapper around snprintf(), I would be in favor of
> >> that.
> >>>>
> >>>> -Bryan
> >>>>
> >>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <solidwallofcode@oath.com
> .
> >> INVALID>
> >>>> wrote:
> >>>>>
> >>>>> I have several pull requests up currently involving updates to output
> >>>>> formatting for BufferWriter. I was asked to provide more detail on
> the
> >>>>> point of these pulls requests. Anyone who is interested can read this
> >>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html
> for
> >>>> that
> >>>>> detail.
> >>>>
> >>>>
> >>
> >>
>
>
Re: Buffer Writer Formatting
Posted by Derek Dagit <de...@oath.com.INVALID>.
+1 for this
There is probably a lot of context from earlier big-picture technical
discussions that I was missing when I tried to review one of the PRs.
Specific examples of code that would be improved or problems that would be
fixed would be really helpful in deciding if the changes make sense for us.
Likewise if there are complexity concerns, more specific examples would be
helpful there too.
On Mon, Apr 23, 2018 at 11:22 AM, Bryan Call <bc...@apache.org> wrote:
> We should have a breakout session on this at the ATS Summit because it
> doesn’t look like this is going to be resolved on the mailing list. We can
> summarize the breakout session on the mailing list and open it up for
> anyone else that wants to add to the discussion.
>
> -Bryan
>
>
> > On Apr 20, 2018, at 7:00 PM, Alan Carroll <so...@oath.com.INVALID>
> wrote:
> >
> > I don't think Leif and Bryan are taking this seriously. For instance, I
> > went and checked the PRs dealing with BufferWriter formatting (3332). I
> > found one. In the same period, there was also a PR to deal with a
> compiler
> > problem with printf() (3476). One of these is dangerously unstable, the
> > other solid and reliable.
> >
> > The code is hardly pushing the compiler technology envelope - it's all
> > C++11 with one small utility class ported from C++14 (which uses only
> C++11
> > features). This is 7 year old technology, it only seems new because we're
> > so far behind.
> >
> > Similarly Bryan complains "On the design, formatting of objects shouldn't
> > be centralized. This creates dependency problems and problems with
> access
> > private members. Each object should have its own way of formatting in a
> > decentralized design." Which is exactly what the design does, lets each
> > type define its own formatting overload function. This isn't an
> incidental
> > feature, it's the core of the work.
> >
> > I do appreciate your concern for my time, but as the person who has (1)
> > done the work and (2) used the library, my view is I expect an overall
> time
> > saving from the endeavor. Frankly, just using it for IP addresses I would
> > consider a win. Even ignoring that, it's a sunk cost now, almost all the
> > work is done.
> >
> > P.S. So, if I say I don't want to replace printf, there's no problem? Of
> > course, no small part of the reason for the work on basic types like
> > integers was to get the performance level Leif and Bryan said was
> required.
> > When Bryan asked me to do the write up for the mailing list, I
> deliberately
> > took out any mention of Debug() in order to focus on the actual work, not
> > future plans. But that wasn't taken seriously either.
> >
> > On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
> >
> >> Traveling, so keeping this short for now...
> >>
> >>> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID>
> >> wrote:
> >>>
> >>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> >>> agree the "extra" value is small for TSDebug. I feel the use of
> >>> bufferwritter is great within our code base.
> >>
> >>
> >> Many of us have spent significant amounts of time dealing with how to
> deal
> >> with he integration of this library on various platforms. I have yet to
> see
> >> the benefits here outweigh the work efforts. I have little confidence
> that
> >> the code would not continue to be a PITA when dealing with cross
> platforms
> >> / compilers. It’s just really pushing the envelope on how compilers
> >> implement these some of these features.
> >>
> >> The other concern I have is that we might be building a library that is
> to
> >> difficult to use,that people wont bother and rather just printf things.
> I
> >> know you fixed the performance issues that were noticeable before, but
> this
> >> is still a complex piece of code that IMO has yet to show value.
> >>
> >> Yes, pretty harsh, and we should have had this discussion long ago. I
> also
> >> know that some of this work was done to deal with string expansion in
> >> header_rewrite; but this is definitely not what I had in mind at the
> time.
> >> I wanted to just to be able to efficiently expand the %{} conditions in
> >> strings, not implement printf as it is.
> >>
> >> Ciao,
> >>
> >> — Leif
> >>>
> >>> -Jason
> >>>
> >>>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
> >>>>
> >>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
> >>>> benefit. Also, I don’t think adding another formatting interface for
> >>>> strings is something we want to maintain or use.
> >>>>
> >>>> The main downside, with snprintf(), I see reading the examples is
> having
> >>>> to keep track of the length and position in the buffer if you are
> >> calling
> >>>> snprintf() multiple times. This can be handled writing a simple
> wrapper
> >>>> around snprintf(), which I have done before in about 20 lines of code.
> >> If
> >>>> we want to expose a wrapper around snprintf(), I would be in favor of
> >> that.
> >>>>
> >>>> -Bryan
> >>>>
> >>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <solidwallofcode@oath.com
> .
> >> INVALID>
> >>>> wrote:
> >>>>>
> >>>>> I have several pull requests up currently involving updates to output
> >>>>> formatting for BufferWriter. I was asked to provide more detail on
> the
> >>>>> point of these pulls requests. Anyone who is interested can read this
> >>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html
> for
> >>>> that
> >>>>> detail.
> >>>>
> >>>>
> >>
> >>
>
>
--
Derek
Re: Buffer Writer Formatting
Posted by Bryan Call <bc...@apache.org>.
We should have a breakout session on this at the ATS Summit because it doesn’t look like this is going to be resolved on the mailing list. We can summarize the breakout session on the mailing list and open it up for anyone else that wants to add to the discussion.
-Bryan
> On Apr 20, 2018, at 7:00 PM, Alan Carroll <so...@oath.com.INVALID> wrote:
>
> I don't think Leif and Bryan are taking this seriously. For instance, I
> went and checked the PRs dealing with BufferWriter formatting (3332). I
> found one. In the same period, there was also a PR to deal with a compiler
> problem with printf() (3476). One of these is dangerously unstable, the
> other solid and reliable.
>
> The code is hardly pushing the compiler technology envelope - it's all
> C++11 with one small utility class ported from C++14 (which uses only C++11
> features). This is 7 year old technology, it only seems new because we're
> so far behind.
>
> Similarly Bryan complains "On the design, formatting of objects shouldn't
> be centralized. This creates dependency problems and problems with access
> private members. Each object should have its own way of formatting in a
> decentralized design." Which is exactly what the design does, lets each
> type define its own formatting overload function. This isn't an incidental
> feature, it's the core of the work.
>
> I do appreciate your concern for my time, but as the person who has (1)
> done the work and (2) used the library, my view is I expect an overall time
> saving from the endeavor. Frankly, just using it for IP addresses I would
> consider a win. Even ignoring that, it's a sunk cost now, almost all the
> work is done.
>
> P.S. So, if I say I don't want to replace printf, there's no problem? Of
> course, no small part of the reason for the work on basic types like
> integers was to get the performance level Leif and Bryan said was required.
> When Bryan asked me to do the write up for the mailing list, I deliberately
> took out any mention of Debug() in order to focus on the actual work, not
> future plans. But that wasn't taken seriously either.
>
> On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
>> Traveling, so keeping this short for now...
>>
>>> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID>
>> wrote:
>>>
>>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
>>> agree the "extra" value is small for TSDebug. I feel the use of
>>> bufferwritter is great within our code base.
>>
>>
>> Many of us have spent significant amounts of time dealing with how to deal
>> with he integration of this library on various platforms. I have yet to see
>> the benefits here outweigh the work efforts. I have little confidence that
>> the code would not continue to be a PITA when dealing with cross platforms
>> / compilers. It’s just really pushing the envelope on how compilers
>> implement these some of these features.
>>
>> The other concern I have is that we might be building a library that is to
>> difficult to use,that people wont bother and rather just printf things. I
>> know you fixed the performance issues that were noticeable before, but this
>> is still a complex piece of code that IMO has yet to show value.
>>
>> Yes, pretty harsh, and we should have had this discussion long ago. I also
>> know that some of this work was done to deal with string expansion in
>> header_rewrite; but this is definitely not what I had in mind at the time.
>> I wanted to just to be able to efficiently expand the %{} conditions in
>> strings, not implement printf as it is.
>>
>> Ciao,
>>
>> — Leif
>>>
>>> -Jason
>>>
>>>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>>>>
>>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>>>> benefit. Also, I don’t think adding another formatting interface for
>>>> strings is something we want to maintain or use.
>>>>
>>>> The main downside, with snprintf(), I see reading the examples is having
>>>> to keep track of the length and position in the buffer if you are
>> calling
>>>> snprintf() multiple times. This can be handled writing a simple wrapper
>>>> around snprintf(), which I have done before in about 20 lines of code.
>> If
>>>> we want to expose a wrapper around snprintf(), I would be in favor of
>> that.
>>>>
>>>> -Bryan
>>>>
>>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <solidwallofcode@oath.com.
>> INVALID>
>>>> wrote:
>>>>>
>>>>> I have several pull requests up currently involving updates to output
>>>>> formatting for BufferWriter. I was asked to provide more detail on the
>>>>> point of these pulls requests. Anyone who is interested can read this
>>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>>>> that
>>>>> detail.
>>>>
>>>>
>>
>>
Re: Buffer Writer Formatting
Posted by Alan Carroll <so...@oath.com.INVALID>.
I don't think Leif and Bryan are taking this seriously. For instance, I
went and checked the PRs dealing with BufferWriter formatting (3332). I
found one. In the same period, there was also a PR to deal with a compiler
problem with printf() (3476). One of these is dangerously unstable, the
other solid and reliable.
The code is hardly pushing the compiler technology envelope - it's all
C++11 with one small utility class ported from C++14 (which uses only C++11
features). This is 7 year old technology, it only seems new because we're
so far behind.
Similarly Bryan complains "On the design, formatting of objects shouldn't
be centralized. This creates dependency problems and problems with access
private members. Each object should have its own way of formatting in a
decentralized design." Which is exactly what the design does, lets each
type define its own formatting overload function. This isn't an incidental
feature, it's the core of the work.
I do appreciate your concern for my time, but as the person who has (1)
done the work and (2) used the library, my view is I expect an overall time
saving from the endeavor. Frankly, just using it for IP addresses I would
consider a win. Even ignoring that, it's a sunk cost now, almost all the
work is done.
P.S. So, if I say I don't want to replace printf, there's no problem? Of
course, no small part of the reason for the work on basic types like
integers was to get the performance level Leif and Bryan said was required.
When Bryan asked me to do the write up for the mailing list, I deliberately
took out any mention of Debug() in order to focus on the actual work, not
future plans. But that wasn't taken seriously either.
On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
> Traveling, so keeping this short for now...
>
> > On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID>
> wrote:
> >
> > Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> > agree the "extra" value is small for TSDebug. I feel the use of
> > bufferwritter is great within our code base.
>
>
> Many of us have spent significant amounts of time dealing with how to deal
> with he integration of this library on various platforms. I have yet to see
> the benefits here outweigh the work efforts. I have little confidence that
> the code would not continue to be a PITA when dealing with cross platforms
> / compilers. It’s just really pushing the envelope on how compilers
> implement these some of these features.
>
> The other concern I have is that we might be building a library that is to
> difficult to use,that people wont bother and rather just printf things. I
> know you fixed the performance issues that were noticeable before, but this
> is still a complex piece of code that IMO has yet to show value.
>
> Yes, pretty harsh, and we should have had this discussion long ago. I also
> know that some of this work was done to deal with string expansion in
> header_rewrite; but this is definitely not what I had in mind at the time.
> I wanted to just to be able to efficiently expand the %{} conditions in
> strings, not implement printf as it is.
>
> Ciao,
>
> — Leif
> >
> > -Jason
> >
> >> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
> >>
> >> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
> >> benefit. Also, I don’t think adding another formatting interface for
> >> strings is something we want to maintain or use.
> >>
> >> The main downside, with snprintf(), I see reading the examples is having
> >> to keep track of the length and position in the buffer if you are
> calling
> >> snprintf() multiple times. This can be handled writing a simple wrapper
> >> around snprintf(), which I have done before in about 20 lines of code.
> If
> >> we want to expose a wrapper around snprintf(), I would be in favor of
> that.
> >>
> >> -Bryan
> >>
> >>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <solidwallofcode@oath.com.
> INVALID>
> >> wrote:
> >>>
> >>> I have several pull requests up currently involving updates to output
> >>> formatting for BufferWriter. I was asked to provide more detail on the
> >>> point of these pulls requests. Anyone who is interested can read this
> >>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
> >> that
> >>> detail.
> >>
> >>
>
>
Re: Buffer Writer Formatting
Posted by Leif Hedstrom <zw...@apache.org>.
> On Apr 20, 2018, at 8:28 PM, Walt Karas <wk...@oath.com.INVALID> wrote:
>
> A broader issue is that Alan had the impression there was a clear
> consensus to do this, and so proceeded to do a lot of work. We should
> understand how that happened.
Agreed, it’s very unfortunate, and I agree that we should focus this discussion on making sure we are more in sync with what we want and expect.
FWIW, I was not aware that the intent was to replace printf when this started.
— Leif
>
> Walt
>
>> On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
>> Traveling, so keeping this short for now...
>>
>>> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID> wrote:
>>>
>>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
>>> agree the "extra" value is small for TSDebug. I feel the use of
>>> bufferwritter is great within our code base.
>>
>>
>> Many of us have spent significant amounts of time dealing with how to deal with he integration of this library on various platforms. I have yet to see the benefits here outweigh the work efforts. I have little confidence that the code would not continue to be a PITA when dealing with cross platforms / compilers. It’s just really pushing the envelope on how compilers implement these some of these features.
>>
>> The other concern I have is that we might be building a library that is to difficult to use,that people wont bother and rather just printf things. I know you fixed the performance issues that were noticeable before, but this is still a complex piece of code that IMO has yet to show value.
>>
>> Yes, pretty harsh, and we should have had this discussion long ago. I also know that some of this work was done to deal with string expansion in header_rewrite; but this is definitely not what I had in mind at the time. I wanted to just to be able to efficiently expand the %{} conditions in strings, not implement printf as it is.
>>
>> Ciao,
>>
>> — Leif
>>>
>>> -Jason
>>>
>>>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>>>>
>>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>>>> benefit. Also, I don’t think adding another formatting interface for
>>>> strings is something we want to maintain or use.
>>>>
>>>> The main downside, with snprintf(), I see reading the examples is having
>>>> to keep track of the length and position in the buffer if you are calling
>>>> snprintf() multiple times. This can be handled writing a simple wrapper
>>>> around snprintf(), which I have done before in about 20 lines of code. If
>>>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>>>
>>>> -Bryan
>>>>
>>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>>>> wrote:
>>>>>
>>>>> I have several pull requests up currently involving updates to output
>>>>> formatting for BufferWriter. I was asked to provide more detail on the
>>>>> point of these pulls requests. Anyone who is interested can read this
>>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>>>> that
>>>>> detail.
>>>>
>>>>
>>
Re: Buffer Writer Formatting
Posted by Walt Karas <wk...@oath.com.INVALID>.
A broader issue is that Alan had the impression there was a clear
consensus to do this, and so proceeded to do a lot of work. We should
understand how that happened.
Walt
On Fri, Apr 20, 2018 at 7:22 PM, Leif Hedstrom <zw...@apache.org> wrote:
> Traveling, so keeping this short for now...
>
>> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID> wrote:
>>
>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
>> agree the "extra" value is small for TSDebug. I feel the use of
>> bufferwritter is great within our code base.
>
>
> Many of us have spent significant amounts of time dealing with how to deal with he integration of this library on various platforms. I have yet to see the benefits here outweigh the work efforts. I have little confidence that the code would not continue to be a PITA when dealing with cross platforms / compilers. It’s just really pushing the envelope on how compilers implement these some of these features.
>
> The other concern I have is that we might be building a library that is to difficult to use,that people wont bother and rather just printf things. I know you fixed the performance issues that were noticeable before, but this is still a complex piece of code that IMO has yet to show value.
>
> Yes, pretty harsh, and we should have had this discussion long ago. I also know that some of this work was done to deal with string expansion in header_rewrite; but this is definitely not what I had in mind at the time. I wanted to just to be able to efficiently expand the %{} conditions in strings, not implement printf as it is.
>
> Ciao,
>
> — Leif
>>
>> -Jason
>>
>>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>>>
>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>>> benefit. Also, I don’t think adding another formatting interface for
>>> strings is something we want to maintain or use.
>>>
>>> The main downside, with snprintf(), I see reading the examples is having
>>> to keep track of the length and position in the buffer if you are calling
>>> snprintf() multiple times. This can be handled writing a simple wrapper
>>> around snprintf(), which I have done before in about 20 lines of code. If
>>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>>
>>> -Bryan
>>>
>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>>> wrote:
>>>>
>>>> I have several pull requests up currently involving updates to output
>>>> formatting for BufferWriter. I was asked to provide more detail on the
>>>> point of these pulls requests. Anyone who is interested can read this
>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>>> that
>>>> detail.
>>>
>>>
>
Re: Buffer Writer Formatting
Posted by Leif Hedstrom <zw...@apache.org>.
Traveling, so keeping this short for now...
> On Apr 20, 2018, at 10:18 AM, Jason Kenny <jk...@oath.com.INVALID> wrote:
>
> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> agree the "extra" value is small for TSDebug. I feel the use of
> bufferwritter is great within our code base.
Many of us have spent significant amounts of time dealing with how to deal with he integration of this library on various platforms. I have yet to see the benefits here outweigh the work efforts. I have little confidence that the code would not continue to be a PITA when dealing with cross platforms / compilers. It’s just really pushing the envelope on how compilers implement these some of these features.
The other concern I have is that we might be building a library that is to difficult to use,that people wont bother and rather just printf things. I know you fixed the performance issues that were noticeable before, but this is still a complex piece of code that IMO has yet to show value.
Yes, pretty harsh, and we should have had this discussion long ago. I also know that some of this work was done to deal with string expansion in header_rewrite; but this is definitely not what I had in mind at the time. I wanted to just to be able to efficiently expand the %{} conditions in strings, not implement printf as it is.
Ciao,
— Leif
>
> -Jason
>
>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>>
>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>> benefit. Also, I don’t think adding another formatting interface for
>> strings is something we want to maintain or use.
>>
>> The main downside, with snprintf(), I see reading the examples is having
>> to keep track of the length and position in the buffer if you are calling
>> snprintf() multiple times. This can be handled writing a simple wrapper
>> around snprintf(), which I have done before in about 20 lines of code. If
>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>
>> -Bryan
>>
>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>> wrote:
>>>
>>> I have several pull requests up currently involving updates to output
>>> formatting for BufferWriter. I was asked to provide more detail on the
>>> point of these pulls requests. Anyone who is interested can read this
>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>> that
>>> detail.
>>
>>
Re: Buffer Writer Formatting
Posted by Bryan Call <bc...@apache.org>.
That is not an issue with modern compliers, like we use in CI:
regex_remap/regex_remap.cc: In function ‘TSRemapStatus TSRemapDoRemap(void*, TSHttpTxn, TSRemapRequestInfo*)’:
regex_remap/regex_remap.cc:1114:18: error: format ‘%g’ expects argument of type ‘double’, but argument 2 has type ‘int’ [-Werror=format=]
TSError("%g", i);
^
-Bryan
> On Apr 20, 2018, at 9:59 AM, Walt Karas <wk...@oath.com.INVALID> wrote:
>
> For me the most important benefit is no undefined behavior / memory
> corruption, like the kind you'd get from:
>
> int i = 10;
> TSError("%g", i);
>
> On Fri, Apr 20, 2018 at 9:18 AM, Jason Kenny <jk...@oath.com.invalid> wrote:
>> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
>> agree the "extra" value is small for TSDebug. I feel the use of
>> bufferwritter is great within our code base.
>>
>> -Jason
>>
>> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>>
>>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>>> benefit. Also, I don’t think adding another formatting interface for
>>> strings is something we want to maintain or use.
>>>
>>> The main downside, with snprintf(), I see reading the examples is having
>>> to keep track of the length and position in the buffer if you are calling
>>> snprintf() multiple times. This can be handled writing a simple wrapper
>>> around snprintf(), which I have done before in about 20 lines of code. If
>>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>>
>>> -Bryan
>>>
>>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>>> wrote:
>>>>
>>>> I have several pull requests up currently involving updates to output
>>>> formatting for BufferWriter. I was asked to provide more detail on the
>>>> point of these pulls requests. Anyone who is interested can read this
>>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>>> that
>>>> detail.
>>>
>>>
Re: Buffer Writer Formatting
Posted by Walt Karas <wk...@oath.com.INVALID>.
For me the most important benefit is no undefined behavior / memory
corruption, like the kind you'd get from:
int i = 10;
TSError("%g", i);
On Fri, Apr 20, 2018 at 9:18 AM, Jason Kenny <jk...@oath.com.invalid> wrote:
> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> agree the "extra" value is small for TSDebug. I feel the use of
> bufferwritter is great within our code base.
>
> -Jason
>
> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>
>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>> benefit. Also, I don’t think adding another formatting interface for
>> strings is something we want to maintain or use.
>>
>> The main downside, with snprintf(), I see reading the examples is having
>> to keep track of the length and position in the buffer if you are calling
>> snprintf() multiple times. This can be handled writing a simple wrapper
>> around snprintf(), which I have done before in about 20 lines of code. If
>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>
>> -Bryan
>>
>> > On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>> wrote:
>> >
>> > I have several pull requests up currently involving updates to output
>> > formatting for BufferWriter. I was asked to provide more detail on the
>> > point of these pulls requests. Anyone who is interested can read this
>> > document - https://solidwallofcode.github.io/buffer-writer.en.html for
>> that
>> > detail.
>>
>>
Re: Buffer Writer Formatting
Posted by Bryan Call <bc...@apache.org>.
There are multiple concerns:
1. More code to maintain and more PRs to review
2. Complexity of the code is increased
3. Learning a new formatting and access to documentation - man snprintf vs reading a webpage
4. Bugs in the implementation that haven’t been found yet, instead of using a standard solution that has been well vetted
5. Opportunity cost of spending time of this instead of fixing more important issues
-Bryan
> On Apr 20, 2018, at 7:18 AM, Jason Kenny <jk...@oath.com.INVALID> wrote:
>
> Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
> agree the "extra" value is small for TSDebug. I feel the use of
> bufferwritter is great within our code base.
>
> -Jason
>
> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>
>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>> benefit. Also, I don’t think adding another formatting interface for
>> strings is something we want to maintain or use.
>>
>> The main downside, with snprintf(), I see reading the examples is having
>> to keep track of the length and position in the buffer if you are calling
>> snprintf() multiple times. This can be handled writing a simple wrapper
>> around snprintf(), which I have done before in about 20 lines of code. If
>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>
>> -Bryan
>>
>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>> wrote:
>>>
>>> I have several pull requests up currently involving updates to output
>>> formatting for BufferWriter. I was asked to provide more detail on the
>>> point of these pulls requests. Anyone who is interested can read this
>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>> that
>>> detail.
>>
>>
Re: Buffer Writer Formatting
Posted by Jason Kenny <jk...@oath.com.INVALID>.
Is the concern bufferwritter or the use fo bufferwritter in TSDebug. I
agree the "extra" value is small for TSDebug. I feel the use of
bufferwritter is great within our code base.
-Jason
On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
> benefit. Also, I don’t think adding another formatting interface for
> strings is something we want to maintain or use.
>
> The main downside, with snprintf(), I see reading the examples is having
> to keep track of the length and position in the buffer if you are calling
> snprintf() multiple times. This can be handled writing a simple wrapper
> around snprintf(), which I have done before in about 20 lines of code. If
> we want to expose a wrapper around snprintf(), I would be in favor of that.
>
> -Bryan
>
> > On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
> wrote:
> >
> > I have several pull requests up currently involving updates to output
> > formatting for BufferWriter. I was asked to provide more detail on the
> > point of these pulls requests. Anyone who is interested can read this
> > document - https://solidwallofcode.github.io/buffer-writer.en.html for
> that
> > detail.
>
>
Re: Buffer Writer Formatting
Posted by Bryan Call <bc...@apache.org>.
The documentation doesn’t mention Debug(), but you have talked that you would like to see this as a complete replacement for Debug(). That also needs to be discussed.
Open source projects (proxies, and https servers) I have looked at use printf() or iostreams. I haven’t seen one that implements is own formatting library.
On the design, formatting of objects shouldn't be centralized. This creates dependency problems and problems with access private members. Each object should have its own way of formatting in a decentralized design.
-Bryan
> On Apr 20, 2018, at 6:27 AM, Alan Carroll <so...@oath.com.INVALID> wrote:
>
> The document doesn't mention Debug() nor TSDebug().
>
> I think the downsides of (1) having to do pre-string conversions for
> non-builtin types such as IP Addresses and (2) having to select from a
> confusing and sometimes (as in PR 3476) impossible to be cross platform
> portable set of integer format codes are much bigger downsides and unlikely
> to be fixed by a length tracking wrapper.
>
> I would say the overall utility of this is demonstrated by the repeated
> attempts to bypass printf, e.g. the entire C++ stream I/O library, and the
> other near-standard Boost with Boost.Format.
>
> I think you are overestimating the maintenance cost. At this point, with
> the built in types written, further work would be done any way, simply with
> different syntactic wrapping. For instance, the IP Address formatting could
> be done via a family of additional ats_ip_XXX functions,
> or even done cut&paste style in multiple locations, but I fail to see
> either of those as less work. Such work is going (or has been) done, it
> would be good to have a mechanism where such work could be done only once.
> My view is the overall maintenance costs of the code base would be reduced.
>
> On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
>
>> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
>> benefit. Also, I don’t think adding another formatting interface for
>> strings is something we want to maintain or use.
>>
>> The main downside, with snprintf(), I see reading the examples is having
>> to keep track of the length and position in the buffer if you are calling
>> snprintf() multiple times. This can be handled writing a simple wrapper
>> around snprintf(), which I have done before in about 20 lines of code. If
>> we want to expose a wrapper around snprintf(), I would be in favor of that.
>>
>> -Bryan
>>
>>> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
>> wrote:
>>>
>>> I have several pull requests up currently involving updates to output
>>> formatting for BufferWriter. I was asked to provide more detail on the
>>> point of these pulls requests. Anyone who is interested can read this
>>> document - https://solidwallofcode.github.io/buffer-writer.en.html for
>> that
>>> detail.
>>
>>
Re: Buffer Writer Formatting
Posted by Alan Carroll <so...@oath.com.INVALID>.
The document doesn't mention Debug() nor TSDebug().
I think the downsides of (1) having to do pre-string conversions for
non-builtin types such as IP Addresses and (2) having to select from a
confusing and sometimes (as in PR 3476) impossible to be cross platform
portable set of integer format codes are much bigger downsides and unlikely
to be fixed by a length tracking wrapper.
I would say the overall utility of this is demonstrated by the repeated
attempts to bypass printf, e.g. the entire C++ stream I/O library, and the
other near-standard Boost with Boost.Format.
I think you are overestimating the maintenance cost. At this point, with
the built in types written, further work would be done any way, simply with
different syntactic wrapping. For instance, the IP Address formatting could
be done via a family of additional ats_ip_XXX functions,
or even done cut&paste style in multiple locations, but I fail to see
either of those as less work. Such work is going (or has been) done, it
would be good to have a mechanism where such work could be done only once.
My view is the overall maintenance costs of the code base would be reduced.
On Thu, Apr 19, 2018 at 5:42 PM, Bryan Call <bc...@apache.org> wrote:
> Replacing Debug()/TSDebug() with BufferWriter/bwformat has little
> benefit. Also, I don’t think adding another formatting interface for
> strings is something we want to maintain or use.
>
> The main downside, with snprintf(), I see reading the examples is having
> to keep track of the length and position in the buffer if you are calling
> snprintf() multiple times. This can be handled writing a simple wrapper
> around snprintf(), which I have done before in about 20 lines of code. If
> we want to expose a wrapper around snprintf(), I would be in favor of that.
>
> -Bryan
>
> > On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID>
> wrote:
> >
> > I have several pull requests up currently involving updates to output
> > formatting for BufferWriter. I was asked to provide more detail on the
> > point of these pulls requests. Anyone who is interested can read this
> > document - https://solidwallofcode.github.io/buffer-writer.en.html for
> that
> > detail.
>
>
Re: Buffer Writer Formatting
Posted by Bryan Call <bc...@apache.org>.
Replacing Debug()/TSDebug() with BufferWriter/bwformat has little benefit. Also, I don’t think adding another formatting interface for strings is something we want to maintain or use.
The main downside, with snprintf(), I see reading the examples is having to keep track of the length and position in the buffer if you are calling snprintf() multiple times. This can be handled writing a simple wrapper around snprintf(), which I have done before in about 20 lines of code. If we want to expose a wrapper around snprintf(), I would be in favor of that.
-Bryan
> On Apr 19, 2018, at 11:20 AM, Alan Carroll <so...@oath.com.INVALID> wrote:
>
> I have several pull requests up currently involving updates to output
> formatting for BufferWriter. I was asked to provide more detail on the
> point of these pulls requests. Anyone who is interested can read this
> document - https://solidwallofcode.github.io/buffer-writer.en.html for that
> detail.