search

acimpoeru / google-glog

Automatically exported from code.google.com/p/google-glog
Other
0 stars 0 forks source link

Performance: Reduce dynamic memory allocations from 3 to 1 per log message. #131

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
Currently, each LOG(INFO) requires 3 memory allocations in LogMessag::Init(): 
one each for LogMessageData, char[kMaxLogMessageLen+1], and LogStream.

It would be logically more consistent to contain all this data in 
LogMessageData; as a side effect there would only be a single allocation per 
log message.   My suggestion is to change LogMessageData from
    char* buf_;
    LogStream* stream_alloc_;
    LogStream* stream_;
to
    char buf_[kMaxLogMessageLen+1];
    LogStream stream_;

This will also lead to somewhat cleaner code for the static instances 
fatal_msg_data_exclusive_ and fatal_msg_data_shared_, because they no longer 
require the additional "static char fatal_msg_buf_exclusive[]" etc.

The suggested change would also require moving the definition of LogMessageData 
from logging.h to logging.cc (requiring kMaxLogMessageLen, which is defined in 
logging.cc).  This move is also reasonable, because LogMessageData is currently 
not accessible as a private inner class anyway.

Would you consider such a change to glog?  I so, I would be happy to provide a 
patch; I have already signed the Google's CLA.

Original issue reported on code.google.com by en...@node.ch on 18 Sep 2012 at 1:00

GoogleCodeExporter commented 8 years ago 
I'll look into this.

Original comment by shinichi...@gmail.com on 9 Jan 2013 at 4:12

GoogleCodeExporter commented 8 years ago 
Great, thank you for looking into this!

I saw that you have already moved LogMessageData (included in glog 0.3.3; 
http://code.google.com/p/google-glog/source/detail?r=120 ) -- so now the actual 
patch will be straightforward.

Original comment by en...@node.ch on 23 May 2013 at 12:17

GoogleCodeExporter commented 8 years ago 
Oops, I moved LogMessageData as the first step of this issue, but I forgot to 
do the remaining tasks. I don't have time to fix this right now. If you can 
provide a patch, I'm very happy to apply it. Thanks!

Original comment by shinichi...@gmail.com on 23 May 2013 at 8:54

GoogleCodeExporter commented 8 years ago 
Sure, I am happy to provide a patch (I have already signed the CLA).  Your 
previous work makes the attached patch pleasantly short:

* Reduce dynamic memory allocations by storing buffer data inside 
LogMessageData.

Original comment by en...@node.ch on 24 May 2013 at 7:11

Attachments:

GoogleCodeExporter commented 8 years ago 
https://code.google.com/p/google-glog/source/detail?r=136

Applied. Thanks for the patch and sorry for the latency.

Original comment by shinichi...@gmail.com on 29 May 2013 at 2:11