Skip to content
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

log/Log.cc: speed optimizations. Merged 3 writes into 1. #6307

Merged
merged 1 commit into from Oct 25, 2015

Conversation

aclamk
Copy link
Contributor

@aclamk aclamk commented Oct 19, 2015

Got rid of std::string construction.
More unification on syslog,stderr,fd.

Signed-off-by: Adam Kupczyk akupczyk@mirantis.com

Got rid of std::string construction.
More unification on syslog,stderr,fd.

Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
@aclamk aclamk changed the title Speed optimizations. Merged 3 writes into 1. log/Log.cc: speed optimizations. Merged 3 writes into 1. Oct 19, 2015
if (r < 0)
cerr << "problem writing to " << m_log_file << ": " << cpp_strerror(r) << std::endl;
}
buflen += e->snprintf(buf + buflen, sizeof(buf) - buflen - 1 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I bet we can do even better with something that looks like pwritev(2)... then we avoid doing a memcpy into buf entirely, and instead construct an iovec array (there should be a max of 3 or 4 distinct inputs). The write or pwritev syscall is doing another copy anyway, so the one into buf shouldn't be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried using writev but it is actually slower.
Below using writev vs using memcpy to make 1 buffer.
size= 6 IOVS=3 time=107768usec
size= 6 WRITE time=84712usec
size= 12 IOVS=3 time=131376usec
size= 12 WRITE time=120907usec
size= 24 IOVS=3 time=183488usec
size= 24 WRITE time=172170usec
size= 48 IOVS=3 time=271790usec
size= 48 WRITE time=271460usec
size= 96 IOVS=3 time=674332usec
size= 96 WRITE time=424097usec
size=192 IOVS=3 time=769107usec
size=192 WRITE time=784972usec
size=384 IOVS=3 time=2253296usec
size=384 WRITE time=1509636usec
size=768 IOVS=3 time=2977487usec
size=768 WRITE time=2974786usec

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It make sense to me that very small vector will result in bad performance. I guess most of logs should be small enough

liewegas added a commit that referenced this pull request Oct 25, 2015
log/Log.cc: speed optimizations. Merged 3 writes into 1.

Reviewed-by: Sage Weil <sage@redhat.com>
@liewegas liewegas merged commit 62b522e into ceph:master Oct 25, 2015
@liewegas
Copy link
Member

Oops.. this passed testing but I merged too soon (and reverted). Two issues:

1- This is putting lot sof \0 in the log output. View a resulting log with 'less' to see.
2- Misc style.. mostly missing whitespace between if and (, on either side of =. Please see CodingStyle.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants