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

Looking into source of logging overhead #85

Closed
houqp opened this issue Oct 8, 2021 · 7 comments
Closed

Looking into source of logging overhead #85

houqp opened this issue Oct 8, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@houqp
Copy link
Member

houqp commented Oct 8, 2021

https://tech.marksblogg.com/roapi-rust-data-api.html mentioned that changing logging level from info to error resulted in 50% speed boost, this is certainly not expected :(

@houqp houqp added the bug Something isn't working label Oct 8, 2021
@Dandandan
Copy link
Contributor

A part might be from sqlparser?
At least it might be better to move most of that logging to debug level instead of info.

@houqp
Copy link
Member Author

houqp commented Oct 10, 2021

@Dandandan I am not seeing info logging from sqlparser, looks like it's only using log::debug at the moment?

@houqp
Copy link
Member Author

houqp commented Oct 10, 2021

After taking a closer look, turns out the benchmark command used in the sample blog post is not using the rest api, not the sql api, so it by passes the sql planner entirely. The rest api arguments are mapped to dataframes directly. I tried removing actix-web's logging middleware and was able to confirm that this is the main source of bottleneck, which is very surprising :D

I am going to prioritize #76 instead instead of digging into the middleware code.

@Dandandan
Copy link
Contributor

Ah yes - good find. Sounds like a good idea 👍

@marklit
Copy link

marklit commented Oct 11, 2021

A few blogs ago I was given a tip to allocate let mut line = String::new(); ahead of time and use a buffer to print out. It had a pretty large impact on the codebase I was working on. You can find the src/main.rs where this technique is used here: https://tech.marksblogg.com/rdns-domain-name-tld-extract-rust.html

I'm not sure how much work it would be to patch Actix versus replace it entirely but I thought I'd throw this out there.

@houqp
Copy link
Member Author

houqp commented Oct 12, 2021

Thanks @marklit for the tip. I will verify the logging overhead once we moved to a new web framework and optimize it if it still turns out to be an issue.

@houqp houqp changed the title Looking source of logging overhead Looking into source of logging overhead Oct 14, 2021
@houqp
Copy link
Member Author

houqp commented Oct 24, 2021

I wrote a custom logger implementation at

pub struct HttpLoggerLayer {}
, which reduces the RPS degradation to about 10% for requests completes in less than 1ms. The overhead should be much less noticeable for real world serving scenarios. So I am closing this issue for now until there is valid case to call for more optimizations.

@houqp houqp closed this as completed Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants