Periodic flush record files instead of flushing them on std::endl #1241
Conversation
|
|
mint570
force-pushed
the
record_flush_upstream
branch
from
9b161f9 to
fbff512
Compare
| @@ -177,7 +177,7 @@ void Recorder::recordLine( | |||
|
|
|||
| if (m_ofstream.is_open()) | |||
| { | |||
| m_ofstream << getTimestamp() << "|" << line << std::endl; | |||
| m_ofstream << getTimestamp() << "|" << line << '\n'; | |||
There was a problem hiding this comment.
this is bad as it can get, please revert
There was a problem hiding this comment.
Why is this bad? This is the main change of this PR. The std::endl will trigger flush. Every line it writes trigger the flush, which impacts performance.
(The PR is not merged, nothing need to revert.)
There was a problem hiding this comment.
yes, we want to trigger flush
There was a problem hiding this comment.
Is there a reason we need to flush on every line? That impacts performance. This PR changes to flush every second. The drawback is that if the program crashes, some record lines might miss.
There was a problem hiding this comment.
yes, reason is to keep logs to last line in case of process crash
PS. make some performance measurements with and without this to check actual gain and present numbers here. normally during warm boot performance is not an issue since very high number of commands are via route entry creations, but many of those operations during warm boot are packed into bulk request like 1k routes per single command, so during warm boot there is about 2-3k log lines logged, i don't think that's much performance impact, but i never measured this, at some point long time ago we achieved 16k routes per second using bulk api so our goal of 10k/sec was reached
There was a problem hiding this comment.
It had about 8% end-to-end improvement on our testing. Since our hardware/test could be very different, this is only for reference.
We have tests that program 1k routes & nexthop groups (with batch size 100, and nexthop group size 5).
Before the change:
Insert: 536 ms
Modify: 339 ms
Delete: 329 ms
After the change (together with sonic-net/sonic-swss#2782):
Insert: 472 ms
Modify: 333 ms
Delete: 299 ms
The modify did not have much improvement.
Also, the std library will auto flush when the buffer is full. In our case, the buffer is about 1k which holds about 10 log lines.
There was a problem hiding this comment.
this is not a significant difference, i would propose to leave code as is
This PR couples with sonic-net/sonic-swss#2782.
This is done for performance purposes. In our testing, this provides 8% end-to-end performance improvement.