CODEWITHSUNDEEP
c++windowsoptimization
Maanu
Maanu

Reputation: 5203

CPU Utilization High

We are using the following method to write the log to log file. The log entries are kept in a vector named m_LogList (stl string entries are kept in the vector). The method is called when the size of the vector is more than 100. The CPU utilization of the Log server is around 20-40% if we call FlushLog method. If we comment out the FlushLog method, the CPU utilisation drops to 10-20% range.
What optimizations can I use to reduce the CPU utilization? We are using fstream object for writing the log entries to file

void CLogFileWriter::FlushLog()
{
    CRCCriticalSectionLock lock(m_pFileCriticalSection);
    //Entire content of the vector are writing to the file
    if(0 < m_LogList.size())
    {       
        for (int i = 0; i < (int)m_LogList.size(); ++i)
        {
            m_ofstreamLogFile << m_LogList[i].c_str()<<endl;
            m_nSize = m_ofstreamLogFile.tellp();

            if(m_pLogMngr->NeedsToBackupFile(m_nSize))
            {
                         // Backup the log file
            }
        }

        m_ofstreamLogFile.flush();
        m_LogList.clear(); //Clearing the content of the Log List           
    }
}

Upvotes: 1

Views: 261

Answers (3)

rodrigo
rodrigo

Reputation: 98496

A few comments:

if(0 < m_LogList.size())

Should be:

if(!m_LogList.empty())

Although with a vector it shouldn't make a difference.

Also you should consider moving the 

m_nSize = m_ofstreamLogFile.tellp();
if(m_pLogMngr->NeedsToBackupFile(m_nSize)) { /*...*/ }

out of the loop. You don't say how much CPU it uses, but I'd bet it is heavy.

You could also iterate using iterators:

for (int i = 0; i < (int)m_LogList.size(); ++i)

Should be:

for (std::vector<std::string>::iterator it = m_LogList.begin(); 
     it != m_LogList.end(); ++it)

Lastly, change the line:

m_ofstreamLogFile << m_LogList[i].c_str()<<endl;

Into:

m_ofstreamLogFile << m_LogList[i] << '\n';

The .c_str() is unnecesary. And the endl writes an EOL and flushes the stream. You do not want to do that, as you are flushing it at the end of the loop.

Upvotes: 0

KillianDS
KillianDS

Reputation: 17186

I'd consider first of all dumping the log in one stdlib call like this:

std::copy(list.begin(), list.end(), std::ostream_iterator<std::string>(m_ofstreamLogFile, "\n"));

This will remove the flushing because of endl and the unnecessary conversion to a c string. CPU-wise this should be quite efficient.

You can do the backup afterwards unless you really care about a very specific limit, but even in that case I'd say: backup atsome lower threshold so that you can account for some overflow.

Also, remove if(0 < m_LogList.size()), it's not really necessary for anything.

Upvotes: 3

MSalters
MSalters

Reputation: 180145

The first optimization I'd use is to drop the .c_str() in << m_LogList[i].c_str(). It forces operator<< to do a strlen (O(n)) instead of relying on string::size (O(1)).

Also, I'd just sum string sizes, instead of calling tellp.

Finally, << endl includes a flush, on every line. Just use << '\n'. You already have the flush at the end.

Upvotes: 3

Related Questions