perf(logging): reduce logging overhead in hot paths#9990
Conversation
- Remove dead debug lambda in PUTFileJob::start() that constructed a temporary QLoggingCategory on every upload progress tick. The signal is already connected to the proper handler on the next line. - Downgrade setFileRecord's 28-field log from qCInfo to qCDebug so it no longer fires in production on every file record update. - Add QtInfoMsg default to 3 Q_LOGGING_CATEGORY declarations that were missing it, preventing debug-level output from being active by default. - Remove tr() and QString::number() allocations from a qCDebug call in usermodel.cpp quota update path. Signed-off-by: Qoole <2862661+qoole@users.noreply.github.com>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
mgallien
left a comment
There was a problem hiding this comment.
thanks
see my inline comment
| qCDebug(lcDb) << "Updating file record for path:" << record.path() << "inode:" << record._inode | ||
| << "modtime:" << record._modtime << "type:" << record._type << "etag:" << record._etag | ||
| << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm.toString() | ||
| << "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader | ||
| << "e2eMangledName:" << record.e2eMangledName() << "isE2eEncrypted:" << record.isE2eEncrypted() | ||
| << "lock:" << (record._lockstate._locked ? "true" : "false") | ||
| << "lock owner type:" << record._lockstate._lockOwnerType | ||
| << "lock owner:" << record._lockstate._lockOwnerDisplayName | ||
| << "lock owner id:" << record._lockstate._lockOwnerId | ||
| << "lock editor:" << record._lockstate._lockEditorApp | ||
| << "sharedByMe:" << record._sharedByMe | ||
| << "isShared:" << record._isShared | ||
| << "lastShareStateFetchedTimestamp:" << record._lastShareStateFetchedTimestamp | ||
| << "isLivePhoto" << record._isLivePhoto | ||
| << "livePhotoFile" << record._livePhotoFile | ||
| << "folderQuota - bytesUsed:" << record._folderQuota.bytesUsed << "bytesAvailable:" << record._folderQuota.bytesAvailable; |
There was a problem hiding this comment.
we often need this log output when debugging an issue an user had in real life
this is the main reason for the info log level
can you revert this change ?
Summary
Reduces logging overhead in several frequently-hit code paths:
PUTFileJob::start(): removes a dead debug lambda that constructed a temporaryQLoggingCategoryon every upload progress tick. The progress signal is already connected to the proper handler on the next line, so the lambda was a no-op that ran on every chunk.SyncJournalDb::setFileRecord: downgrades the 28-field log line fromqCInfotoqCDebugso it no longer fires in production on every file record update. With large syncs this generates a substantial amount of log spam at info level.Q_LOGGING_CATEGORYdefaults: addsQtInfoMsgdefault to three category declarations that were missing it, preventing debug-level output from being active by default in those modules.usermodel.cppquota update: removestr()andQString::number()allocations from aqCDebugcall that runs on every quota refresh, even when debug logging is not enabled.Checklist
AI (if applicable)