Re: Merging out changes from Facebook

[David Reiss <dr...@facebook.com>]

I'm committing these now.

On 09/26/2010 01:23 PM, Bryan Duxbury wrote:
> Yeah, I'll probably cut the 0.5 RC branch tomorrow.
> 
> On Sun, Sep 26, 2010 at 8:47 AM, David Reiss <dr...@facebook.com> wrote:
> 
>>> I'd like to ask that this be scheduled for 0.6, with the intention of
>>> wrapping up 0.5 ASAP and moving into development of 0.6.
>> That's fine.  We're cutting the branch in a few days anyway, right?
>>
>>> The asio vs libev async implementations is sort of like different
>>> flavors of MQ transports.  Would be nice to try to support the
>>> different flavors in as clean a way as possible.
>> This is designed to be really easy.  To implement a server, all you need
>> to do is drive a TAsyncBufferProcessor.  To implement a client, all you
>> need is an implementation of TAsyncChannel.  If you look at these
>> interfaces, I hope you will agree that they are very minimal and easy to
>> integrate into any async framework.
>>
>> --David
>>
>> On 09/25/2010 10:00 PM, Bruce Lowekamp wrote:
>>>
>>> I'd like to request holding off on them until after 0.5.
>>>
>>> I'm hoping to get time and permission to begin contributing some changes
>> we have.  One of them is a boost::asio/boost::bind based async c++
>> implementation.  My biggest concern with the implementation has been the
>> number (and invasive nature) of the changes to t_cpp_generator.  Looking
>> (very briefly) at the changes you have, there's more abstraction than
>> before, and some of it is definitely in the right direction, but I need to
>> look at it some more to figure out how much of what we need is there and
>> whether there are any conflicts.
>>>
>>> Biggest reason to ask that it not be in 0.5, though, is that if it's in
>> 0.5, we essentially won't be able to try any release candidates with our
>> code---the changes will be too dramatic.   Would need more time to work on
>> merging out changes before we could test.
>>>
>>> The asio vs libev async implementations is sort of like different flavors
>> of MQ transports.  Would be nice to try to support the different flavors in
>> as clean a way as possible.
>>>
>>> I'd like to ask that this be scheduled for 0.6, with the intention of
>>> wrapping up 0.5 ASAP and moving into development of 0.6.
>>>
>>> Bruce
>>>
>>>
>>> On Sep 25, 2010, at 9:16 PM, Anthony Molinaro wrote:
>>>
>>>> I agree, especially since I don't actually use the C++ stuff :).  But
>>>> seriously, I know the pain of maintaining a long lived branch and the
>>>> new features provided sound promising.  So I'd say just commit it so
>>>> it makes it into 0.5.0, so much is changing with 0.5.0 anyway, might
>>>> as well add in some more :).
>>>>
>>>> -Anthony
>>>>
>>>> On Sat, Sep 25, 2010 at 06:51:12PM -0700, Bryan Duxbury wrote:
>>>>> I say go for it. I wonder if it's even worth publishing the git branch
>> and
>>>>> waiting for complaints - history has shown that we probably won't get
>> them
>>>>> until way after the fact.
>>>>>
>>>>> -Bryan
>>>>>
>>>>> On Sat, Sep 25, 2010 at 5:41 PM, David Reiss <dr...@facebook.com>
>> wrote:
>>>>>
>>>>>> Hey guys,
>>>>>>
>>>>>> We've been doing a lot of experimental changes to Thrift/C++ within
>>>>>> Facebook.  Some of our experiments have panned out and some haven't,
>> but
>>>>>> some have dragged on for a long time with half-finished code checked
>> into
>>>>>> trunk.  This has prevented us from merging our changes back out to
>> Apache
>>>>>> for a while, which has been very frustrating.  I've cleaned up a lot
>> of
>>>>>> the code as best I can, and I'm hoping the community would be okay
>>>>>> accepting these patches into the trunk, even though some of the
>> features
>>>>>> are still experimental and likely to have interface changes.  Because
>> of
>>>>>> the interleaved development history, it's not really possible to tease
>> out
>>>>>> many of the individual features without significant conflict
>> resolution.
>>>>>> However, I'm hoping that no one will be tempted to do that since all
>> of
>>>>>> the changes should be isolated to new APIs and fully
>> backwards-compatible
>>>>>> at the source level.
>>>>>>
>>>>>> The main changes are:
>>>>>>
>>>>>> - Templated transport and protocol code.  This change has sped up some
>> of
>>>>>> our benchmarks by 5x for serialization and 3x for deserialization, and
>>>>>> reduced the end-to-end latency of at least one of our production
>> systems
>>>>>> by 50%.  All that's needed is a few type annotations in your C++
>> source.
>>>>>> - Async client and server support.  So far, just implemented as an
>>>>>> evhttp-based client and server, but the infrastructure is in place for
>>>>>> writing fully event-driven Thrift clients and servers.
>>>>>> - Hooks that allow user code to get better statistics about the calls
>>>>>> being made to the server.
>>>>>> - Better test coverage for transport classes.
>>>>>> - Various small features and bug fixes.
>>>>>>
>>>>>> My plan is to create JIRA issues with patches for the 5 items above
>>>>>> (including a conglomo-issue for the small changes), publish a git
>> branch
>>>>>> with all of the changes (plus an instant release if people want), then
>>>>>> wait to see if anyone complains and commit if they don't.
>>>>>>
>>>>>> The git branch is called "fb-merge-p2" and is available at either of
>>>>>> git://git.thrift-rpc.org/thrift.git
>>>>>> git://github.com/dreiss/thrift.git
>>>>>>
>>>>>> Non git users can pull the full combined patch from r996610 at
>>>>>>
>>>>>>
>> http://gitweb.thrift-rpc.org/?p=thrift.git;a=treediff_plain;h=refs/heads/pri/dreiss/fb-merge-p2;hp=d5c342b
>>>>>>
>>>>>> The full list of changes is at
>>>>>>
>>>>>>
>> http://gitweb.thrift-rpc.org/?p=thrift.git;a=log;h=refs/heads/pri/dreiss/fb-merge-p2;hb=HEAD
>>>>>> (click through on "commitdiff" to see the full individual commits.)
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>> --David
>>>>>>
>>>>
>>>> --
>>>> ------------------------------------------------------------------------
>>>> Anthony Molinaro                           <anthonym@alumni.caltech.edu
>>>
>>>
>>
>