You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@thrift.apache.org by Bruce Simpson <bm...@incunabulum.net> on 2009/11/24 20:25:56 UTC

[PATCH] use std::string::assign() in TBinaryProtocol::readString() path

Hi,

I noticed TCompactProtocol is using std::string::assign(). 
TBinaryProtocol assigns to the passed-in str as an lvalue.

This may make only a minor difference -- although GNU libstdc++ 
refcounts std::basic_string's internal representation, which might save 
a copy on the input path.

cheers,
BMS

Re: [PATCH] use std::string::assign() in TBinaryProtocol::readString() path

Posted by Bryan Duxbury <br...@rapleaf.com>.
Bruce,

How would you feel about opening a JIRA ticket to track this  
suggestion? That's our usual workflow.

-Bryan

On Nov 24, 2009, at 11:25 AM, Bruce Simpson wrote:

> Hi,
>
> I noticed TCompactProtocol is using std::string::assign().  
> TBinaryProtocol assigns to the passed-in str as an lvalue.
>
> This may make only a minor difference -- although GNU libstdc++  
> refcounts std::basic_string's internal representation, which might  
> save a copy on the input path.
>
> cheers,
> BMS
> Index: TBinaryProtocol.cpp
> ===================================================================
> --- TBinaryProtocol.cpp	(revision 883543)
> +++ TBinaryProtocol.cpp	(working copy)
> @@ -387,7 +387,7 @@
>      string_buf_size_ = size;
>    }
>    trans_->readAll(string_buf_, size);
> -  str = string((char*)string_buf_, size);
> +  str.assign((char*)string_buf_, size);
>    return (uint32_t)size;
>  }
>


Re: [PATCH] use std::string::assign() in TBinaryProtocol::readString() path

Posted by Bruce Simpson <bm...@incunabulum.net>.
...gmake check ok w/svn rev 883543 from trunk