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 <dre...@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 <antho...@alumni.caltech.edu>
