New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: remove redundant codes in rgw_cache.h #13902

Merged
merged 1 commit into from Mar 21, 2017

Conversation

Projects
None yet
3 participants
@Wilhelmshaven
Contributor

Wilhelmshaven commented Mar 9, 2017

I'm not sure whether the old way is faster or not, seems unnecessary to do that with unsafe sprintf.

Signed-off-by: lihongjie lihongjie@cmss.chinamobile.com

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 9, 2017

rebase please?

@Wilhelmshaven

This comment has been minimized.

Contributor

Wilhelmshaven commented Mar 10, 2017

@liewegas rebased, tkx

@cbodley cbodley self-assigned this Mar 16, 2017

@cbodley

This comment has been minimized.

Contributor

cbodley commented Mar 16, 2017

I'm not sure whether the old way is faster or not, seems unnecessary to do that with unsafe sprintf.

i agree that we shouldn't be using sprintf(), thanks. but this pattern of std::string::append() could lead to extra allocations. could you add some math to calculate the resulting string length, and use std::string::reserve() to make sure we only allocate once? something like this, maybe:

std::string buf;
buf.reserve(pool.name.size() + oid.size() + 2);
return buf.append(pool.name).append('+').append(oid);
@Wilhelmshaven

This comment has been minimized.

Contributor

Wilhelmshaven commented Mar 20, 2017

@cbodley Fixed. Thanks for your really helpful advise :)

return string(buf);
std::string buf;
buf.reserve(pool.name.size() + oid.size() + 2);
return buf.append(pool.name).append("+").append(oid);

This comment has been minimized.

@cbodley

cbodley Mar 20, 2017

Contributor

oops sorry, this last line will interfere with return value optimization, and induce an extra alloc/copy back to the caller. splitting up the append()s and return will help:

    buf.append(pool.name).append("+").append(oid);
    return buf; // on separate line for return value optimization

This comment has been minimized.

@Wilhelmshaven

Wilhelmshaven Mar 21, 2017

Contributor

Oh, okay, fixed, thx. :)

rgw: remove redundant codes in rgw_cache.h
Signed-off-by: lihongjie <lihongjie@cmss.chinamobile.com>
@cbodley

This comment has been minimized.

Contributor

cbodley commented Mar 21, 2017

@cbodley cbodley merged commit 5444c9d into ceph:master Mar 21, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@Wilhelmshaven Wilhelmshaven deleted the Wilhelmshaven:rm_redundant_code branch Mar 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment